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.

Sign up for GitHub

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

Issue warnings if braces are missing #7270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b419803
Issue warnings if code is indented too far to the left
odersky Sep 19, 2019
12a9190
Issue warnings if code is indented too far to the right
odersky Sep 19, 2019
59bb86b
Turn misindent error into warning
odersky Sep 19, 2019
46e1afe
Warning if code after an empty template body is indented
odersky Sep 19, 2019
cf3b9b0
Fix indentation
odersky Sep 19, 2019
3e2ab41
Fix indentation in test
odersky Sep 19, 2019
bdf9b81
Fix test again
odersky Sep 19, 2019
25af394
Polishings
odersky Sep 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 76 additions & 32 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,50 @@ object Parsers {
try op finally in.adjustSepRegions(closing)
}

/** Parse `body` while checking (under -noindent) that a `{` is not missing before it.
* This is done as follows:
* If the next token S is indented relative to the current region,
* and the end of `body` is followed by a new line and another statement,
* check that that other statement is indented less than S
*/
def subPart[T](body: () => T): T = in.currentRegion match
case r: InBraces if in.isAfterLineEnd =>
val startIndentWidth = in.indentWidth(in.offset)
if r.indentWidth < startIndentWidth then
// Note: we can get here only if indentation is not significant
// If indentation is significant, we would see an <indent> as current token
// and the indent region would be Indented instead of InBraces.
//
// If indentation would be significant, an <indent> would be inserted here.
val t = body()
// Therefore, make sure there would be a matching <outdent>
def nextIndentWidth = in.indentWidth(in.next.offset)
if (in.token == NEWLINE || in.token == NEWLINES)
&& !(nextIndentWidth < startIndentWidth)
then
warning(
if startIndentWidth <= nextIndentWidth then
i"""Line is indented too far to the right, or a `{' is missing before:
|
|$t"""
else
in.spaceTabMismatchMsg(startIndentWidth, nextIndentWidth),
in.next.offset
)
t
else body()
case _ => body()

/** If indentation is not significant, check that this is not the start of a
* statement that's indented relative to the current region.
*/
def checkNextNotIndented(): Unit = in.currentRegion match
case r: InBraces if in.token == NEWLINE || in.token == NEWLINES =>
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 •
edited
Loading

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.


/* -------- REWRITES ----------------------------------------------------------- */

/** The last offset where a colon at the end of line would be required if a subsequent { ... }
Expand Down Expand Up @@ -1606,13 +1650,6 @@ object Parsers {

/* ----------- EXPRESSIONS ------------------------------------------------ */

/** EqualsExpr ::= `=' Expr
*/
def equalsExpr(): Tree = {
accept(EQUALS)
expr()
}

def condExpr(altToken: Token): Tree =
if (in.token == LPAREN) {
var t: Tree = atSpan(in.offset) { Parens(inParens(exprInParens())) }
Expand Down Expand Up @@ -1676,7 +1713,9 @@ object Parsers {
*/
val exprInParens: () => Tree = () => expr(Location.InParens)

def expr(): Tree = expr(Location.ElseWhere)
val expr: () => Tree = () => expr(Location.ElseWhere)

def subExpr() = subPart(expr)

def expr(location: Location.Value): Tree = {
val start = in.offset
Expand Down Expand Up @@ -1714,7 +1753,7 @@ object Parsers {
atSpan(in.skipToken()) {
val cond = condExpr(DO)
newLinesOpt()
val body = expr()
val body = subExpr()
WhileDo(cond, body)
}
}
Expand Down Expand Up @@ -1753,7 +1792,7 @@ object Parsers {
if (in.token == CATCH) {
val span = in.offset
in.nextToken()
(expr(), span)
(subExpr(), span)
}
else (EmptyTree, -1)

Expand All @@ -1768,7 +1807,7 @@ object Parsers {
}

val finalizer =
if (in.token == FINALLY) { in.nextToken(); expr() }
if (in.token == FINALLY) { in.nextToken(); subExpr() }
else {
if (handler.isEmpty) warning(
EmptyCatchAndFinallyBlock(body),
Expand Down Expand Up @@ -1823,7 +1862,7 @@ object Parsers {
case EQUALS =>
t match {
case Ident(_) | Select(_, _) | Apply(_, _) =>
atSpan(startOffset(t), in.skipToken()) { Assign(t, expr()) }
atSpan(startOffset(t), in.skipToken()) { Assign(t, subExpr()) }
case _ =>
t
}
Expand Down Expand Up @@ -1870,8 +1909,8 @@ object Parsers {
atSpan(start, in.skipToken()) {
val cond = condExpr(THEN)
newLinesOpt()
val thenp = expr()
val elsep = if (in.token == ELSE) { in.nextToken(); expr() }
val thenp = subExpr()
val elsep = if (in.token == ELSE) { in.nextToken(); subExpr() }
else EmptyTree
mkIf(cond, thenp, elsep)
}
Expand Down Expand Up @@ -2224,7 +2263,7 @@ object Parsers {
else if (in.token == CASE) generator()
else {
val pat = pattern1()
if (in.token == EQUALS) atSpan(startOffset(pat), in.skipToken()) { GenAlias(pat, expr()) }
if (in.token == EQUALS) atSpan(startOffset(pat), in.skipToken()) { GenAlias(pat, subExpr()) }
else generatorRest(pat, casePat = false)
}

Expand All @@ -2241,7 +2280,7 @@ object Parsers {
if (casePat) GenCheckMode.FilterAlways
else if (ctx.settings.strict.value) GenCheckMode.Check
else GenCheckMode.FilterNow // filter for now, to keep backwards compat
GenFrom(pat, expr(), checkMode)
GenFrom(pat, subExpr(), checkMode)
}

/** ForExpr ::= `for' (`(' Enumerators `)' | `{' Enumerators `}')
Expand Down Expand Up @@ -2313,12 +2352,12 @@ object Parsers {
newLinesOpt()
if (in.token == YIELD) {
in.nextToken()
ForYield(enums, expr())
ForYield(enums, subExpr())
}
else if (in.token == DO) {
if (rewriteToOldSyntax()) dropTerminator()
in.nextToken()
ForDo(enums, expr())
ForDo(enums, subExpr())
}
else {
if (!wrappedEnums) syntaxErrorOrIncomplete(YieldOrDoExpectedInForComprehension())
Expand Down Expand Up @@ -2758,7 +2797,7 @@ object Parsers {
syntaxError(VarValParametersMayNotBeCallByName(name, mods.is(Mutable)))
val tpt = paramType()
val default =
if (in.token == EQUALS) { in.nextToken(); expr() }
if (in.token == EQUALS) { in.nextToken(); subExpr() }
else EmptyTree
if (impliedMods.mods.nonEmpty)
impliedMods = impliedMods.withMods(Nil) // keep only flags, so that parameter positions don't overlap
Expand Down Expand Up @@ -3003,7 +3042,7 @@ object Parsers {
(lhs.toList forall (_.isInstanceOf[Ident])))
wildcardIdent()
else
expr()
subExpr()
}
else EmptyTree
lhs match {
Expand Down Expand Up @@ -3043,7 +3082,7 @@ object Parsers {
if (in.isScala2Mode) newLineOptWhenFollowedBy(LBRACE)
val rhs = {
if (!(in.token == LBRACE && scala2ProcedureSyntax(""))) accept(EQUALS)
atSpan(in.offset) { constrExpr() }
atSpan(in.offset) { subPart(constrExpr) }
}
makeConstructor(Nil, vparamss, rhs).withMods(mods).setComment(in.getDocComment(start))
}
Expand Down Expand Up @@ -3076,7 +3115,7 @@ object Parsers {
if (in.token == EQUALS)
indentRegion(name) {
in.nextToken()
expr()
subExpr()
}
else if (!tpt.isEmpty)
EmptyTree
Expand All @@ -3100,7 +3139,7 @@ object Parsers {
/** ConstrExpr ::= SelfInvocation
* | `{' SelfInvocation {semi BlockStat} `}'
*/
def constrExpr(): Tree =
val constrExpr: () => Tree = () =>
if (in.isNestedStart)
atSpan(in.offset) {
inBracesOrIndented {
Expand Down Expand Up @@ -3351,7 +3390,7 @@ object Parsers {
if in.token == EQUALS && parents.length == 1 && parents.head.isType then
in.nextToken()
mods1 |= Final
DefDef(name, tparams, vparamss, parents.head, expr())
DefDef(name, tparams, vparamss, parents.head, subExpr())
else
//println(i"given $name $hasExtensionParams $hasGivenSig")
possibleTemplateStart()
Expand Down Expand Up @@ -3431,22 +3470,27 @@ object Parsers {

/** TemplateOpt = [Template]
*/
def templateOpt(constr: DefDef): Template = {
def templateOpt(constr: DefDef): Template =
possibleTemplateStart()
if (in.token == EXTENDS || isIdent(nme.derives))
if in.token == EXTENDS || isIdent(nme.derives) then
template(constr)
else
if (in.isNestedStart) template(constr)
else Template(constr, Nil, Nil, EmptyValDef, Nil)
}
if in.isNestedStart then
template(constr)
else
checkNextNotIndented()
Template(constr, Nil, Nil, EmptyValDef, Nil)

/** TemplateBody ::= [nl] `{' TemplateStatSeq `}'
*/
def templateBodyOpt(constr: DefDef, parents: List[Tree], derived: List[Tree]): Template = {
def templateBodyOpt(constr: DefDef, parents: List[Tree], derived: List[Tree]): Template =
val (self, stats) =
if (in.isNestedStart) templateBody() else (EmptyValDef, Nil)
if in.isNestedStart then
templateBody()
else
checkNextNotIndented()
(EmptyValDef, Nil)
Template(constr, parents, derived, self, stats)
}

def templateBody(): (ValDef, List[Tree]) = {
val r = inDefScopeBraces { templateStatSeq() }
Expand Down
47 changes: 25 additions & 22 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,31 +497,29 @@ object Scanners {
canStartStatTokens.contains(token) &&
!isLeadingInfixOperator())
insert(if (pastBlankLine) NEWLINES else NEWLINE, lineOffset)
else if (indentIsSignificant)
if (nextWidth < lastWidth
|| nextWidth == lastWidth && (indentPrefix == MATCH || indentPrefix == CATCH) && token != CASE)
currentRegion match {
case r: Indented
if !r.isOutermost &&
!isLeadingInfixOperator() &&
!statCtdTokens.contains(lastToken) =>
currentRegion = r.enclosing
insert(OUTDENT, offset)
handleEndMarkers(nextWidth)
case _ =>
}
else if (lastWidth < nextWidth ||
lastWidth == nextWidth && (lastToken == MATCH || lastToken == CATCH) && token == CASE) {
if (canStartIndentTokens.contains(lastToken)) {
else if indentIsSignificant then
if nextWidth < lastWidth
|| nextWidth == lastWidth && (indentPrefix == MATCH || indentPrefix == CATCH) && token != CASE then
if !currentRegion.isOutermost &&
!isLeadingInfixOperator() &&
!statCtdTokens.contains(lastToken) then
currentRegion match
case r: Indented =>
currentRegion = r.enclosing
insert(OUTDENT, offset)
handleEndMarkers(nextWidth)
case r: InBraces if !closingRegionTokens.contains(token) =>
ctx.warning("Line is indented too far to the left, or a `}' is missing",
source.atSpan(Span(offset)))
case _ =>

else if lastWidth < nextWidth
|| lastWidth == nextWidth && (lastToken == MATCH || lastToken == CATCH) && token == CASE then
if canStartIndentTokens.contains(lastToken) then
currentRegion = Indented(nextWidth, Set(), lastToken, currentRegion)
insert(INDENT, offset)
}
}
else if (lastWidth != nextWidth)
errorButContinue(
i"""Incompatible combinations of tabs and spaces in indentation prefixes.
|Previous indent : $lastWidth
|Latest indent : $nextWidth""")
errorButContinue(spaceTabMismatchMsg(lastWidth, nextWidth))
currentRegion match {
case Indented(curWidth, others, prefix, outer) if curWidth < nextWidth && !others.contains(nextWidth) =>
if (token == OUTDENT)
Expand All @@ -535,6 +533,11 @@ object Scanners {
}
}

def spaceTabMismatchMsg(lastWidth: IndentWidth, nextWidth: IndentWidth) =
i"""Incompatible combinations of tabs and spaces in indentation prefixes.
|Previous indent : $lastWidth
|Latest indent : $nextWidth"""

def observeIndented(unless: BitSet, unlessSoftKW: TermName = EmptyTermName): Unit =
if (indentSyntax && isAfterLineEnd && token != INDENT) {
val newLineInserted = token == NEWLINE || token == NEWLINES
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/parsing/Tokens.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ object Tokens extends TokensCommon {

final val statCtdTokens: BitSet = BitSet(THEN, ELSE, DO, CATCH, FINALLY, YIELD, MATCH)

final val closingRegionTokens = BitSet(RBRACE, CASE) | statCtdTokens

final val canStartIndentTokens: BitSet =
statCtdTokens | BitSet(COLONEOL, EQUALS, ARROW, LARROW, WHILE, TRY, FOR)
// `if` is excluded because it often comes after `else` which makes for awkward indentation rules TODO: try to do without the exception
Expand Down
11 changes: 5 additions & 6 deletions compiler/src/dotty/tools/dotc/plugins/Plugins.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,11 @@ trait Plugins {

private var _plugins: List[Plugin] = _
def plugins(implicit ctx: Context): List[Plugin] =
if (_plugins == null) {
_plugins = loadPlugins
_plugins
}
else _plugins

if (_plugins == null) {
_plugins = loadPlugins
_plugins
}
else _plugins

/** A description of all the plugins that are loaded */
def pluginDescriptions: String =
Expand Down
Loading