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

Improve error reporting: Warn when pipe operators are incorrectly indented #1442

Open
Tracked by #1103
rmunn opened this issue Aug 16, 2016 · 3 comments
Open
Tracked by #1103
Labels
Feature Improvement Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.
Milestone

Comments

@rmunn
Copy link

rmunn commented Aug 16, 2016

What

Another one for the #1103 list. A recent F# question on StackOverflow highlighted a place where the compiler's error messages are not helpful. The user wrote the following code:

let process_args argv =
    let (result, _) = ((default_settings, "-f"), argv)
        ||> Array.fold (fun (state, setting) elem ->
            // (snip body of folder function as it's irrelevant here)
        )
    result

Note that the user's original code used the verbose-syntax usage of let ... in here, which I've replaced with light-syntax let (without in) since the syntax makes no difference in this case and the same unhelpful compiler errors appear.

This function produces two error messages: one on the let of let (result, _) = ..., and one on the ||> operator. The error on let is:

Incomplete value or function definition. If this is in an expression, the body of the expression must be indented to the same column as the 'let' keyword.

and the error on the ||> is:

Unexpected infix operator in binding. Expected incomplete structured construct at or before this point or other token.

Why

The StackOverflow user asking this question appeared to understand F# pretty well, but he was not able to figure out from those two error messages that he had an indentation problem with his pipe operator. It should have been indented far enough to be under the ((default_settings, "-f"), argv) tuple, but the error messages on let and ||> give no clue to that fact.

How

The code pattern of let something = somethingElse |> someFunc is extremely common. If the |> operator (or any other pipe operator like ||>, <|, and so on) is found "alone" on a line when the previous line had a let statement, it's likely that the user meant to indent the |> to line up with somethingElse. So a nicer error message for the original user's code might have been:

Unexpected infix operator in binding.
The ||> operator should be indented so that it is aligned under the value it operates on.
Instead of this:

    let (result, _) = ((default_settings, "-f"), argv)
        ||> Array.fold (fun (state, setting) elem ->

you probably meant to write this:

    let (result, _) =
        ((default_settings, "-f"), argv)
        ||> Array.fold (fun (state, setting) elem ->

or possibly this:

    let (result, _) = ((default_settings, "-f"), argv)
                      ||> Array.fold (fun (state, setting) elem ->

With, of course, the actual operator in the sentence "The ||> operator should be indented ...". This will usually be the |> operator, since ||> has very few good use cases outside of piping into a .fold -- but the same error message should be applicable to all the piping operators.

@cartermp cartermp added this to the Unknown milestone Aug 25, 2018
@dsyme dsyme added the Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language. label Sep 16, 2021
@KathleenDollard
Copy link

This would be cool, based on these rules if they work:

If the |> operator (or any other pipe operator like ||>, <|, and so on) is found "alone" on a line when the previous line had a let statement, it's likely that the user meant to indent the |> to line up with somethingElse

Unless Fantomas would pick this up, in which case, perhaps leave it to Fantomas.

I'm not sure it needs as much specificity as required. In this case, at least for a start, perhaps something more hard coded:

Unexpected infix operator in binding.
The ||> operator should be indented so that it is aligned under the value it operates on. Similar to:

    let varName = 
        expression
        ||> f

Although if we go that generic route, the naming deserves more attention. It just seems hard to use the programmers own code in all cases.

Also, would Fantomas catch this?

@smoothdeveloper
Copy link
Contributor

It just seems hard to use the programmers own code in all cases.

This is probably true in current state of the compiler, but not true for many compilers (haskell, rust, clang do report error message with pointing to precise snippets of code in fancy ways (ASCII & all).

If infrastructure is made for this kind of "quoting" the failing code, F# could be potentially as good as those other compilers when reporting error messages with quoted code.

@smoothdeveloper
Copy link
Contributor

Also, would Fantomas catch this?

You may try it online there: https://fsprojects.github.io/fantomas-tools/

for short snippet like this, it puts everything on a single line.

For a snippet like this:

let f hasToBeVeryVeryVeryVeryVeryVeryVeryLong thisOneTooVeryVeryVeryVeryLong =
  thisOneTooVeryVeryVeryVeryLong 
      ||> hasToBeVeryVeryVeryVeryVeryVeryVeryLong

It does the right thing by indenting the line with pipe properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Improvement Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.
Projects
Archived in project
Development

No branches or pull requests

7 participants