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 unnecessary parentheses analyzer & code fix #1235

Merged
merged 10 commits into from
Feb 19, 2024

Conversation

brianrourkeboll
Copy link
Contributor

Note: this will need a small update to use dotnet/fsharp#16461 when FSC 8.0.300 is shipped.

@baronfel
Copy link
Contributor

Awesome - thanks for doing the parallel work here!

@baronfel
Copy link
Contributor

How recent of an 8.0.300 does the 'fixed' version need? There's a 'nightly' branch we use to stage these kinds of fixes that pulls from the nightly FCS feeds that you could do the work on once this is merged into main, and main is merged into nightly.

@@ -80,6 +80,7 @@
// | Lint of file: string<LocalPath> * warningsWithCodes: Lint.EnrichedLintWarning list
| UnusedDeclarations of file: string<LocalPath> * decls: range[] * version: int
| SimplifyNames of file: string<LocalPath> * names: SimplifyNames.SimplifiableRange[] * version: int
| UnnecessaryParentheses of file: string<LocalPath> * ranges: range[] * version: int

Check notice

Code scanning / Ionide.Analyzers.Cli

Detect if generic type should be in the postfix position. Note

Prefer postfix syntax for arrays.
@brianrourkeboll
Copy link
Contributor Author

brianrourkeboll commented Feb 18, 2024

How recent of an 8.0.300 does the 'fixed' version need?

Any version that has dotnet/fsharp#16462 and dotnet/fsharp#16461 in it.

The update would basically look like this:

-let! unnecessaryParentheses = UnnecessaryParentheses.getUnnecessaryParentheses getSourceLine tyRes.GetAST
+let unnecessaryParentheses =
+    (HashSet Range.comparer, tyRes.GetAST)
+    ||> ParsedInput.fold (fun ranges path node ->
+        match node with
+        | SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; rightParenRange = Some _; range = range)) when
+            not (SynExpr.shouldBeParenthesizedInContext getSourceLine path inner)
+            ->
+            ignore (ranges.Add range)
+            ranges
+
+        | SyntaxNode.SynPat(SynPat.Paren(inner, range)) when not (SynPat.shouldBeParenthesizedInContext path inner) ->
+            ignore (ranges.Add range)
+            ranges
+
+        | _ -> ranges)

src/FsAutoComplete.Core/Utils.fs Show resolved Hide resolved
test/FsAutoComplete.Tests.Lsp/FindReferencesTests.fs Outdated Show resolved Hide resolved
@baronfel
Copy link
Contributor

@brianrourkeboll let me know if you want to move this out of draft - I'm happy to merge it.

@brianrourkeboll brianrourkeboll marked this pull request as ready for review February 19, 2024 14:28
@brianrourkeboll
Copy link
Contributor Author

@brianrourkeboll let me know if you want to move this out of draft - I'm happy to merge it.

Yeah I just wanted to be sure the CI passed to catch any dumb oversights I might have made.

@baronfel
Copy link
Contributor

Thanks for this contribution!

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

Successfully merging this pull request may close these issues.

2 participants