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

False positive warning on nested if statements #85

Open
ksromanov opened this issue Oct 11, 2021 · 2 comments
Open

False positive warning on nested if statements #85

ksromanov opened this issue Oct 11, 2021 · 2 comments

Comments

@ksromanov
Copy link

Describe the bug
Camelot does not recognize else if ... pattern, which is widely used to chain conditional expressions. This pattern does not reduce readability of the code.

To Reproduce

let name_of_the_winter_month month =
   if month == 0 then
      "January"
   else if month == 1 then
      "February"
   else if month == 11 then
      "December"
   else if month < 5 && month > 0 then
      "It is spring!"
   else
      "Unknown"

Expected behavior
Camelot should recognize this pattern (chained else if) and issue no warning.

Desktop (please complete the following information):

  • OS: Linux
  • OCaml version: 4.12.1
  • Dune version : 2.8.5
  • Camelot version : 1.7.0
@ksromanov ksromanov changed the title False positive warning on the nested if statements False positive warning on nested if statements Oct 11, 2021
@ksromanov
Copy link
Author

ksromanov commented Oct 11, 2021

Perhaps something like

        | Pexp_ifthenelse (_, bthen, Some (Pexp_ifthenelse  _) as belse) ->
          if depth = 1 then true else
            find_nesting ((skip_seq_let bthen).pexp_desc) (depth - 1) ||
            find_nesting ((skip_seq_let belse).pexp_desc) depth

before https://github.com/upenn-cis1xx/camelot/blob/master/lib/style/verbose.ml#L47 should work.

@hongchangwu
Copy link

hongchangwu commented Oct 11, 2021

On a related note, sometimes it's desirable to write cascading match statements, such as

let output =
  match do_something input with
  | Some output -> output
  | None ->
  match fallback1 input with
  | Some output -> ouput
  | None ->
  match fallback2 input with
  | Some output -> output
  | None -> failwith "Giving up"

I consider this to be idiomatic and more readable than alternatives such as using higher-order functions or introducing helper functions. Unfortunately Camelot flags this as match statements nested too deep.

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

2 participants