Analyzer & code fix for ~IDE0047~ FS3583: remove unnecessary parentheses

[brianrourkeboll]

(See #15408.)

Edit: I ended up using FS3583 as the diagnostic ID in #16079 for now, due to the current limitations called out in #16079 (comment). That means that most of the questions below are moot for the time being.

We should add a code fix for removing unnecessary parentheses, i.e., a fix for the IDE0047 diagnostic—see https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047-ide0048. This implies that we also need an analyzer that emits that diagnostic.

While such a thing could in theory be provided by a separate tool like Fantomas instead—ocamlformat offers similar functionality, for example—I think it probably makes sense to add the logic for it to the F# compiler service, which the VS tooling, Fantomas, etc., can then use.

I have a provisional implementation of the analyzer and code fix in #16079.

Questions

IDE0047 and IDE0048 already exist along with documentation and implementations for C# and VB. Some of the .editorconfig preferences, and the assumptions behind them, don't have obvious answers when applied to F#, since F# supports user-defined custom operators. For example, while the existing documentation for IDE0047 lists specific sets of binary operators to which the various preferences apply, (1) some of these operators have a different shape in F# (e.g., >>> instead of >> for right shift), and (2) in F#, infix operators with an arbitrary number of leading . or ? characters and an arbitrary number of valid trailing punctuation characters are parsed with the same precedence and associativity as the first one or two characters (depending).

• Should we support dotnet_style_parentheses_in_arithmetic_binary_operators? If so, what does this mean in F#? Exactly the "arithmetic binary operators" defined in FSharp.Core? Including the nullable versions? Or exactly the symbols **, *, /, %, +, -, <<<, >>>, &&&, ^^^, |||? Or anything that trims to these?
• Should we support dotnet_style_parentheses_in_relational_binary_operators? If so, what does this mean in F#? Exactly the "relational binary operators" defined in FSharp.Core? Including the nullable versions? Or exactly the symbols >, <, <=, >=, =, <>? Or anything that trims to these?
• Should we support dotnet_style_parentheses_in_other_binary_operators? If so, what does this mean in F#? It at least partially depends on how the previous questions are answered.
• Should we support dotnet_style_parentheses_in_other_operators? If so, what does this mean in F#?
• Should we support IDE0048? Note that this would likely require its own separate logic, although we do already have FS0597.
• Should the docs at https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047-ide0048 be updated (e.g., "Applicable languages") if Analyzer & code fix for ~IDE0047~ FS3583: remove unnecessary parentheses #16079 is merged?
• Should the code fix be aware of other .editorconfig preferences, e.g., anything related to spaces?
  • Probably not in the VS code fix at least (Analyzer & code fix for ~IDE0047~ FS3583: remove unnecessary parentheses #16078 (comment)): VS itself wouldn't normally be aware of Fantomas-specific settings like fsharp_space_after_semicolon ([(1); 2] → [1; 2] versus [1 ; 2]), etc.

[psfinaki]

Hi @brianrourkeboll - I think it would be cool to have this in F#.

We'll need to discuss the implementation probably since AFAIU Fantomas does take .editorconfig into the account so we need to be careful. @nojaf what are your thoughts?

[brianrourkeboll]

We'll need to discuss the implementation probably since AFAIU Fantomas does take .editorconfig into the account so we need to be careful. @nojaf what are your thoughts?

To be fair, there are existing code fixes that don't necessarily heed style config: ImplementInterfaceCodeFixProvider isn't aware of Fantomas's fsharp_multiline_bracket_style, fsharp_space_before_colon, or fsharp_space_before_member, for example.

Luckily the spacing question is pretty minor and can be part of the fix itself, not the logic that determines whether parens are needed in the first place.

[nojaf]

While such a thing could in theory be provided by a separate tool like Fantomas instead

Fantomas historically lacked stability for this feature. Nowadays, it might be more feasible, but I'd prefer to delegate it to a commercial product to avoid a flood of bug reports.

Regarding Fantomas settings, they're likely irrelevant in this case. I don't believe Fantomas is part of the vanilla product, so there's no need to burden @brianrourkeboll with it in this PR. He's doing an excellent job, and we shouldn't overwhelm him with stylistic concerns. We can address this in future PRs.