Parens: more fixes #16901
Merged
Parens: more fixes #16901
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Match-like exprs in `if` exprs, `while` exprs, and `for` exprs. Also `let` exprs. * Nested, dangling `as` patterns. * Outlaw `match` exprs (where the first `|` is leftward of the `m` in `match) * Single-line comments (`//`, `///`). Multiline comments (`(*…*)`) would be… rather more difficult to handle. * Double-parenthesized tuples in method applications, since we can't tell purely syntactically whether the tuple might be the first parameter of a method whose next parameter is an implied outref parameter: `x.TryGetValue ((y, z))` i.e., `x.TryGetValue ((y, z), &value)` * Multiline tuple patterns in `let`-bindings. These need parens if the bound expression starts on the same column.
* Sometimes we can't tell just by looking at the untyped AST whether we need parens, since their necessity may be dictated by type information located elsewhere. Compare, e.g., dotnet#16254, which probably has a similar underlying cause.
❗ Release notes required
|
brianrourkeboll
commented
Mar 19, 2024
brianrourkeboll
commented
Mar 19, 2024
vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnnecessaryParentheses.fs
Outdated
Show resolved
Hide resolved
* E.g., removing parens in place from ```fsharp Debug.Assert((xT.DeclaringType :?> ProvidedTypeDefinition).BelongsToTargetModel) ``` would result in the the argument to `Assert` becoming `(xT.DeclaringType :?> ProvidedTypeDefinition)`. The `.BelongsToTargetModel` would then be parsed as a get on the return value of _that_ call.
T-Gro
approved these changes
Mar 21, 2024
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
brianrourkeboll
added a commit
to brianrourkeboll/FsAutoComplete
that referenced
this pull request
May 7, 2024
See dotnet/fsharp#16901 * Keep parens around outlaw `match` exprs (where the first `|` is leftward of the start of the `match` keyword). * Ignore single-line comments when determining offsides lines. * Don't add a space when removing parens when doing so would result in reparsing an infix op as a prefix op.
TheAngryByrd
pushed a commit
to ionide/FsAutoComplete
that referenced
this pull request
May 8, 2024
See dotnet/fsharp#16901 * Keep parens around outlaw `match` exprs (where the first `|` is leftward of the start of the `match` keyword). * Ignore single-line comments when determining offsides lines. * Don't add a space when removing parens when doing so would result in reparsing an infix op as a prefix op.
TheAngryByrd
added a commit
to ionide/FsAutoComplete
that referenced
this pull request
May 9, 2024
* Shift multiline paren contents less aggressively (#1242) * Shift multiline paren contents less aggressively * Make it actually work * Disambiguate AsSpan overload * Add some code fixes for type mismatch. (#1250) * Migrate FAKE to Fun.Build (#1256) * Migrate FAKE to Fun.Build * Add default Build pipeline. * Purge it with fire (#1255) * Bump analyzers and Fantomas (#1257) * Add empty, disabled tests for go-to-def on C# symbol scenario * fix unicode characters in F# compiler diagnostic messages (#1265) * fix unicode chars in F# compiler diagnostic messages * fix typo in ShadowedTimeouts focused tests * fixup! fix unicode chars in F# compiler diagnostic messages * remove focused tests... * remove debug prints Co-authored-by: Jimmy Byrd <[email protected]> --------- Co-authored-by: Jimmy Byrd <[email protected]> * - remove an ignored call to protocolRangeToRange (#1266) - remove an ignored instance of StreamJsonRpcTracingStrategy * Place XML doc lines before any attribute lists (#1267) * Don't generate params for explicit getters/setters as they are flagged invalid by the compiler (#1268) * bump ProjInfo to the next version to get support for loading broken projects and loading traversal projects (#1270) * only allow one GetProjectOptionsFromScript at a time (#1275) ionide/ionide-vscode-fsharp#2005 * changelog for v0.72.0 * changelog for v0.72.1 * Use actualRootPath instead of p.RootPath when peeking workspaces. (#1278) This fix the issue where rootUri was ignored when using AutomaticWorkspaceInit. * changelog for v0.72.2 * Add support for Cancel a Work Done Progress (#1274) * Add support for Cancel WorkDoneProgress * Fix up saving cancellation * package the tool for .NET 8 as well (#1281) * update changelogs * Adds basic OTel Metric support to fsautocomplete (#1283) * Some parens fixes (#1286) See dotnet/fsharp#16901 * Keep parens around outlaw `match` exprs (where the first `|` is leftward of the start of the `match` keyword). * Ignore single-line comments when determining offsides lines. * Don't add a space when removing parens when doing so would result in reparsing an infix op as a prefix op. * Run LSP tests sequenced (#1287) I'm tired of all the weird failures because of parallelism --------- Co-authored-by: Brian Rourke Boll <[email protected]> Co-authored-by: Florian Verdonck <[email protected]> Co-authored-by: Krzysztof Cieślak <[email protected]> Co-authored-by: MrLuje <[email protected]> Co-authored-by: dawe <[email protected]> Co-authored-by: Chet Husk <[email protected]> Co-authored-by: oupson <[email protected]> Co-authored-by: Chet Husk <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another followup to #16079.
Description
letexprs inifexprs,whileexprs, andforexprs.aspatterns.matchexprs, i.e., where the first|is leftward of theminmatch./////when determining offsides lines. (Multiline comments(*…*)technically pose the same problem, but handling them (and multiline strings, ifdefs, etc.) would require a different approach.)letbindings when removing them would parse differently.Good news: I can now successfully apply "fix all" to the entire FSharp.Compiler.Service project — with ~6,600 lines updated in 155 files — and it still builds successfully after:
Checklist