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

Align implementation with spec of soft modifiers #15961

Merged
merged 1 commit into from
Sep 5, 2022

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Sep 4, 2022

The spec in https://dotty.epfl.ch/docs/reference/soft-modifier.html says:

A soft modifier is treated as potential modifier of a definition if it is followed by a hard modifier or a keyword combination starting a definition (def, val, var, type, given, class, trait, object, enum, case class, case object). Between the two words there may be a sequence of newline tokens and soft modifiers.

But the implementation recognized also soft modifiers in front of just case, not followed by class or object.
The change caused some breakage for enum cases, where we have case alone, but that one cannot be preceded by any
soft modifiers anyway. So only neg tests were affected.

Fixes #15960

The spec in https://dotty.epfl.ch/docs/reference/soft-modifier.html says:

> A soft modifier is treated as potential modifier of a definition if it is followed by a hard modifier or a keyword combination starting a definition (def, val, var, type, given, class, trait, object, enum, case class, case object). Between the two words there may be a sequence of newline tokens and soft modifiers.

But the implementation recognized also soft modifiers in front of just `case`, not followed by `class` or `object`.
The change caused some breakage for enum cases, where we have `case` alone, but that one cannot be preceded by any
soft modifiers anyway. So only neg tests were affected.

Fixes scala#15960
@odersky
Copy link
Contributor Author

odersky commented Sep 4, 2022

@tgodzik I am not sure about the consequences for Metals. The problem I could see is where we type something like

open case 

and want to continue with class. Will the IDE get confused at this point?

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

From Metals perspective we might need to improve the error in Scalameta, but the presentation compiler features should work with whatever compiler provides us. We don't have any specific rules to completions after open case, but might be good to add a test case for it.

@odersky odersky merged commit c93098b into scala:main Sep 5, 2022
@odersky odersky deleted the fix-15960 branch September 5, 2022 11:22
tgodzik added a commit to tgodzik/scalameta that referenced this pull request Sep 16, 2022
tgodzik added a commit to tgodzik/scalameta that referenced this pull request Sep 16, 2022
@Kordyjan Kordyjan added this to the 3.2.2 milestone Aug 1, 2023
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.

open is not treated as a normal identifier inside match
3 participants