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

RedundantBraces for val/var/type cannot be configured after #3022 #3026

Closed
neko-kai opened this issue Jan 4, 2022 · 5 comments · Fixed by #3031
Closed

RedundantBraces for val/var/type cannot be configured after #3022 #3026

neko-kai opened this issue Jan 4, 2022 · 5 comments · Fixed by #3031

Comments

@neko-kai
Copy link

neko-kai commented Jan 4, 2022

version = 3.3.1

rewrite.rules = [RedundantBraces, RedundantParens]
rewrite.redundantBraces.methodBodies = false // remove braces only in interpolations
rewrite.redundantBraces.generalExpressions = false // remove braces only in interpolations
rewrite.redundantBraces.includeUnitMethods = false
rewrite.redundantBraces.stringInterpolation = true
rewrite.redundantBraces.parensForOneLineApply = true

Problem

The intention of the above configuration was to remove braces only in interpolations and single-line applications, however after #3022 val/var/type defs braces are also modified - but without an option to disable it as for def defs.

Expectation

Expected rewrite.redundantBraces.methodBodies to also affect val/var. Often the choice between val & def affects only performance, not semantics, so it's unexpected for braces to change when switching between them.

Proposal

However, methodBodies option cannot logically affect type definitions. The solutions, in order of increasing complexity, could be:

  • Simplistic: Rename methodBodies option to definitions. Deprecate & alias old name.

  • Simple: Add defnBodies option alongside methodBodies.

  • Complex: Remodel option into an array similar to danglingParentheses.exclude:

    rewrite.redundantBraces.exclude = [def, val, var, type, macro]
    // default: rewrite.redundantBraces.exclude = []
    
@kitbellew
Copy link
Collaborator

i think you assumed that this rule is fully configurable, but it's not. there are some constructs that will be rewritten if the rule is enabled, regardless of any other parameter.

as to method bodies, this parameter should have applied to methods with parameters only, otherwise they look like variables, as you surmised.

@neko-kai
Copy link
Author

neko-kai commented Jan 4, 2022

Well, it was configurable enough for the above up until #3022. Dropping the rule entirely would allow upgrading without a diff, but would not restore the ability to specify the style of preferring braces for multi-line definitions (wheter def or val), but removing otherwise clearly redundant braces.

@kitbellew
Copy link
Collaborator

multi-line seems to be the key. perhaps what you need is: https://scalameta.org/scalafmt/docs/configuration.html#inserting-braces

@neko-kai
Copy link
Author

neko-kai commented Jan 4, 2022

@kitbellew
You're right, thank you! Setting

rewrite.redundantBraces.maxLines = -1 // `0` still causes diffs oddly enough

successfully removed the diff on upgrading.

@neko-kai neko-kai closed this as completed Jan 4, 2022
@kitbellew
Copy link
Collaborator

@kitbellew You're right, thank you! Setting

rewrite.redundantBraces.maxLines = -1 // `0` still causes diffs oddly enough

successfully removed the diff on upgrading.

glad you found what you were looking for.

about 0: because the way maxLines was originally implemented really means max line breaks.

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 a pull request may close this issue.

2 participants