Skip to content

Issue warnings if braces are missing #7270

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

Merged
merged 8 commits into from
Sep 23, 2019
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Sep 19, 2019

Issue warnings if brace structure does not correspond to indentation structure.

There are three cases.

Case 1: In a brace-delimited region, if there would be an <outdent> between
one line and the next, a warning is now issued that states:

Line is indented too far to the left or a `}' is missing

This is very helpful for finding missing closing braces. It prevents errors like:

if (x < 0) {
  println(1)
  println(2)

println("done")

Case 2: If significant indentation is turned off (i.e. under -noindent) and we are at the start
of an indented sub-part (i.e. an <indent> would have been inserted under significant indentation),
and the indented part ends in a newline, check that the next statement starts at an indentation width less than the sub-part. This prevents errors like

if (x < 0)
  println(1)
  println(2)

println("done")

Case 3: Under -noindent, warn if code that follows an empty template is
indented more deeply than the template header. Inserting braces
automatically would change meaning in this case.

Example:

  trait A
    case class B() extends A  // error: Line is indented too far to the right
    case object C extends A   // error: Line is indented too far to the right

In a brace-delimited region, if there would be an "unindent" between
one line and the next, a warning is now issued that states:

    Line is indented too far to the left or a `}' is missing

This is very helpful in finding missing closing braces.

The tests are fixed to avoid the warning except for triple-quoted-expr.scala.
The latter has a """ at the start of a line.
Also, fix two more tests
Under -noindent, warn id code that follows an empty template is
indented more deeply than the template header. Inserting braces
automatically would change meaning in this case.

Example:

```scala
  trait A
    case class B() extends A  // error: Line is indented too far to the right
    case object C extends A   // error: Line is indented too far to the right
```
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM

I'm thinking if we could strengthen the rules for infix operators. Currently, the following program runs with no problem:

@main def Test = {
  val a = 6
  var b = 0
  if (a > 5) then
      b = a
- 5

  assert(b == 1)
}

If would be good to check that indentation of infix operator should be bigger than that of currentRegion.outer?

val nextIndentWidth = in.indentWidth(in.next.offset)
if r.indentWidth < nextIndentWidth then
warning(i"Line is indented too far to the right, or a `{' is missing", in.next.offset)
case _ =>
Copy link
Contributor

@liufengyun liufengyun Sep 23, 2019

Choose a reason for hiding this comment

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

The check does not handle code at top-level:

trait A
    case class B() extends A
    case object C extends A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll add a fix to the follow-up PR.

@liufengyun liufengyun assigned odersky and unassigned liufengyun Sep 23, 2019
@odersky
Copy link
Contributor Author

odersky commented Sep 23, 2019

If would be good to check that indentation of infix operator should be bigger than that of currentRegion.outer?

Yes, maybe. But I am not sure the compiler has to do it. Currently the compiler only enforces those indentation rules that make sure that braces around well-indented code are optional. To do more,
we'd have to formulate the rules and come up with an algorithm to enforce them. This might need several cycles to get right, or maybe there won't be an agreement what is the correct formulation of these rules..

@odersky odersky merged commit 0d34bfb into scala:master Sep 23, 2019
@odersky odersky deleted the check-indent branch September 23, 2019 12:46
mbovel added a commit that referenced this pull request May 21, 2025
This PR change the condition to emit the warning “Line is indented too
far to the right, or a `{` or `:` is missing” introduced in #7270 to
only emit it if we are inside an `Indented` region. This fixes #21749.
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.

2 participants