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

Parens: only check for sensitive indentation when really needed #16973

Merged

Conversation

brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Apr 1, 2024

Another followup to #16079 (and #16461).

Description

  • Only check for sensitive indentation when actually needed, i.e., when there is a possibility that the inner construct is valid in context with parentheses but would be invalid in context without parentheses. This enables easier use of the SynExpr.shouldBeParenthesizedInContext API for hypothetical scenarios where the SynExpr being passed in is not already parenthesized; see Ignore expression code fix ionide/FsAutoComplete#1253 (comment).

Notes

  • I have not considered every aspect of the "would this expression need to be parenthesized if it were in this hypothetical context?" use-case, and it is likely that there are other scenarios where the current SynExpr.shouldBeParenthesizedInContext API would behave imperfectly if used for that. I am only addressing one fairly obvious and easily mitigated shortcoming in this PR.
  • The set of "indentation-sensitive" constructs is not perfectly homogeneous — some of these constructs allow the (unparenthesized) inner offsides line to match the outer, while others require that the inner be rightward of the outer by at least one space — but the expense of treating them individually does not seem warranted.

Checklist

  • Test cases added.
  • Release notes entry updated.

Copy link
Contributor

github-actions bot commented Apr 1, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/8.0.300.md

* Bindings in records and anonymous records aren't actually represented
  as bindings in the AST...
@brianrourkeboll brianrourkeboll marked this pull request as ready for review April 1, 2024 17:04
@brianrourkeboll brianrourkeboll requested a review from a team as a code owner April 1, 2024 17:04
@vzarytovskii vzarytovskii enabled auto-merge (squash) April 2, 2024 07:36
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants