Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lints detected in fp2020/2021/2022/2023 programming course #3

Open
19 of 83 tasks
Kakadu opened this issue Aug 19, 2021 · 0 comments
Open
19 of 83 tasks

Add lints detected in fp2020/2021/2022/2023 programming course #3

Kakadu opened this issue Aug 19, 2021 · 0 comments

Comments

@Kakadu
Copy link
Owner

Kakadu commented Aug 19, 2021

  • Exposed public functions but not used anywhere.
  • match ... with _ as x -> ....
  • Algebraic data types
  • Booleans
    • An action that panics on if true then ...
    • if ... then true else false
  • Strings
    • Expression ... ^ ... ^ ... may not be efficient
  • Matching
    • | Foo x when x = true -> ... ~~~> | Foo true -> ....

    • Instead of matching a pair recommend 'let' #13

      match scru with 
      | (a,b) -> rhs

      into

      let (a,b) = scru in rhs
    • Match on bool when scrutinee starts with not

       match not (if_object (get_var_ctx_from_ctx i env).value) with
             | true -> ....        
             | false -> ....
      
    • Rewrite fun e -> match e with ... to function ... if possible

      • Propose function here too
        (fun l ->
         match l with
         | [ ty ] -> ty
         | _ -> t_tuple l)
        
    • Use match instead of if x = WasReturn then ... else if x = WasContinue then .. else ... (related to the lint "use_match_instead_of_equality" which checks only for lists, options, etc...) #27

    • | Return rexpr_o -> ( match rexpr_o with ... )

    • Recommend or-patterns when possible

    • Detect same RHSes

      match MapString.find_opt id env9 with
      | None -> return (MapString.add id v env9)
      | Some _ -> return (MapString.add id v env9) in

      See also Warn about logical or between identical arguments. #42

    • Recommend nested matching

         | EConst x ->
           (match x with
           | CInt _ -> return (Subst.empty, TInt)
           | CBool _ -> return (Subst.empty, TBool)
           | CString _ -> return (Subst.empty, TString))
            match stmts with
            | s :: st -> (
                match s with
                | RETURN ret_e -> (
                    match ret_e with
                    | LITERAL CVOID -> is_correct_void st (r && true)
                    | _ -> is_correct_void st (r && false))
                | _ -> is_correct_void st r)
            | _ -> r
      match n with
      | "malloc" -> (
          match vals with
          | ...)
      | "free" -> (
          match vals with
          | ...)
      | "sizeof" -> (
          match vals with
          | ...)
      | "main" -> ...
  • Exceptions
    • failwith
    • Using of List.hd_exn may be error-prone
    • Matching with _ exception
    • Catching Failure _ exception
    • Suggest function name with _exn suffix when function can throw an exception.
  • Records
    • Rewrite { e=f.e; g=f.g; h=189 } to { f with h = 189 } #10
    • Rewrite { key = fkey; foo = _; bar = _ } to { key = fkey; _ }
    • Rewrite '{ field=field; ... }' but maybe it is not possible and ocamlformat should do it
      • Propose record punning (???)
          let add_func_in_ctx i a s ctx =
            { ctx with funcs = { id = i; args = a; stmts = s } :: ctx.funcs }
        
      • Discourage clashing fields, which require explicit type annotation
         type var_ctx =
           { id : id
           ; value : value
           }
        
         type func_ctx =
           { id : id
           ; args : id list
           ; stmts : statement list
           }
        
  • Monads
    • Adding stuff to Monad signature that doesn't belong there

    • Propose applicatives

               ac >>= fun ac ->
               to_type ctx v >>= fun t -> return @@ (t :: ac))
    • Simplify artifacts from pretty printting  #48

      let* env, typ = check_type env hd in
           return (env, typ)
      
         let new_res = ln ^ Printf.sprintf "%s -> %s " k (exval_to_str v) in
         new_res
  • Format strings: rewrite "\"%s\"" to "%S" #53
  • Think about applying eta-conversion (although it is not correct in general case) #46
          let rec go acc =
            lift2 (fun f x -> f acc x) op (choice [ pl >>= go; pr ]) <|> return acc
          in
          pl >>= go
    It looks like patterns like (fun arg1...argN -> IDENT arg1 ... argN) suites and only them
  • Rewrite '(fun state decl -> eval_dec state decl)' with eta-conversion
  • Expressions like ... :: [ ... ] smell bad
  • Weird stuff
    • let ctx = ctx in ...
    • return @@ Vchar v we should probably recommend parenthesis
    • simplify >>= fun ident -> return ident and fun (ctx', pls) -> return (ctx', pls)
    • lists
      • ... :: [ ... ] smells bad
      • (t' :: ts') @ acc smells bad. Propose t' :: (ts' @ ac)
  • Parser-combinator stuff:
    • Simplify 'token "#" >> token "#"'
  • Global mutable variables
    • adding [@@deriving sexp] introduce code with assignments.
  • Warn when value's name has starting '_' but used elsewhere
  • Detect functions that not used
  • Ignore without specifying type of ignored variable
    • ignore as last expression of function (which doesn't really ignore anything)
    • maybe restrict ignoring of monadic values
  • Detect functions already present in Stdlib and Base. Maybe only declared as structure items.
    • The same for exceptions
    • In worst case detect functions that look like standard list functions: map and fold
  • Rewrite common idiom
    • 'concat' + 'map' => concat_map
    • 'filter' + 'map' => filter_map
    • List.to_seq [ eff ] => Seq.return eff
    • etc...
  • Rewrite fun cd -> let f ... = (* no use of cd *) in ... to let f ... = ... in fun cd -> ...
  • Toplevel print_string ... should be let () = ...
  • Лишние @@ в return @@ Nil
    • Не очень понятно как детектировать гадости типа return @@ (x+y)
    • Но вот тут понятно как: return @@ (get_var_ctx_from_ctx i e_env).value
  • Mutable variables never assigned
  • Unneeded mutual recursion #17
  • Copypaste detection
  • Propose string literals as {| ... |} when a normal string literal has too many '\n'
  • cram tests
    • Detect errornously disabled test (no two spaces in the beginning)
    • recommend (cram (deps ....)
  • (fun (ptrn, _) -> ptrn) -> fst
  • shorten
    let add map k v =
     match Map.add map ~key:k ~data:v with
     ...
    
Kakadu added a commit that referenced this issue Aug 19, 2021
Kakadu added a commit that referenced this issue Aug 19, 2021
Kakadu added a commit that referenced this issue Aug 21, 2021
@Kakadu Kakadu pinned this issue Feb 17, 2022
@Kakadu Kakadu changed the title Add lints detected in fp2020 programming course Add lints detected in fp2020/2021 programming course Mar 21, 2022
@Kakadu Kakadu changed the title Add lints detected in fp2020/2021 programming course Add lints detected in fp2020/2021/2022 programming course Mar 17, 2023
@nemakin nemakin unpinned this issue Oct 26, 2023
@Kakadu Kakadu changed the title Add lints detected in fp2020/2021/2022 programming course Add lints detected in fp2020/2021/2022/2023 programming course Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant