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

Fix #8256: Disallow silent indent for template bodies #8264

Merged
merged 6 commits into from Feb 10, 2020

Conversation

@odersky
Copy link
Contributor

odersky commented Feb 9, 2020

The new syntax requires a :. The old syntax was still supported
under a Config option which was on by default. But the test is further
confirmation that silent identation without a specific leading token
is a bad idea.

odersky added 2 commits Feb 9, 2020
The new syntax requires a `:`. The old syntax was still supported
under a Config option which was on by default. But the test is further
confirmation that silent identation without a specific leading token
is a bad idea.
@odersky odersky force-pushed the dotty-staging:fix-#8256 branch from e118d4a to ef1ff9d Feb 9, 2020
@odersky

This comment has been minimized.

Copy link
Contributor Author

odersky commented Feb 9, 2020

The actual change are just two lines, one in Parser the other in Config. But a lot of tests and infrastructure code had already switched to the shorthand, even though it was "officially" supported for only one release window, which was some months back. I guess that's natural, there's a strong tendency to pick the shortest form available.

@smarter

This comment has been minimized.

Copy link
Member

smarter commented Feb 9, 2020

It still seems weird that the object was detected as more indented than the trait when they were indented the same amount, couldn't this happen in other situations ?

odersky added 2 commits Feb 9, 2020
@odersky

This comment has been minimized.

Copy link
Contributor Author

odersky commented Feb 9, 2020

: It still seems weird that the object was detected as more indented than the trait when they were indented the same amount, couldn't this happen in other situations ?

I don't think so. We have for each statement sequence an indentation width, which is the indentation of the first statement in the sequence. Following statements can be on the right of that width. That's what happened here: The trait was indented too far to the right, and the folllowing statement was interpreted as indented as well, but now indented relative to the trait because it was to the right of the previous indentation width. With an explicit : this cannot happen. What can happen is this:

val a = ???
  trait T:
  val x = 1

Here, again the definition of x would be treated as part of trait T. But there is a clear indication (the :) that this is intended. Wr could make this an error by checking the indentation width of the previous statement but I'd like to observe real world usage a bit more to see whether this is really a problem.

@nicolasstucki nicolasstucki linked an issue that may be closed by this pull request Feb 10, 2020
@odersky odersky merged commit fa2d063 into lampepfl:master Feb 10, 2020
2 checks passed
2 checks passed
CLA User signed CLA
Details
continuous-integration/drone/pr Build is passing
Details
@odersky odersky deleted the dotty-staging:fix-#8256 branch Feb 10, 2020
@anatoliykmetyuk anatoliykmetyuk added this to the 0.23.0-RC1 milestone Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.