Skip to content

Consider syntax with significant indentation #2488

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
29 changes: 27 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/NavigateAST.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import core.Decorators._
import util.Positions._
import Trees.{MemberDef, DefTree}

/** Utility functions to go from typed to untyped ASTs */
/** Utility functions to go from typed to untyped ASTs
* and to navigate in untyped ASTs
*/
object NavigateAST {

/** The untyped tree corresponding to typed tree `tree` in the compilation
Expand Down Expand Up @@ -57,7 +59,6 @@ object NavigateAST {
def untypedPath(pos: Position)(implicit ctx: Context): List[Positioned] =
pathTo(pos, ctx.compilationUnit.untpdTree)


/** The reverse path from node `from` to the node that closest encloses position `pos`,
* or `Nil` if no such path exists. If a non-empty path is returned it starts with
* the node closest enclosing `pos` and ends with `from`.
Expand All @@ -79,4 +80,28 @@ object NavigateAST {
else path
singlePath(from, Nil)
}

/** The subtrees of `roots` that immediately precede `offset`, from outer to inner.
* Every returned subtree `t` satisfies the following condition
*
* - t's position is a non-empty, non-synthetic interval
* - t's end position <= offset` and
* - there is no other eligible tree t' which lies entirely between t's end position and `offset`.
*/
def precedingTrees(offset: Int, roots: List[Positioned])(implicit ctx: Context): List[Positioned] = {
def precedingPath(xs: List[Any], path: List[Positioned]): List[Positioned] = xs match {
case (p: Positioned) :: xs1
if p.pos.exists && p.pos.start < p.pos.end && p.pos.start <= offset =>
val children = p.productIterator.toList.reverse
if (!p.pos.isSynthetic && p.pos.end <= offset) precedingPath(children, p :: path)
else precedingPath(children ::: xs1, path)
case (ys: List[_]) :: xs1 =>
precedingPath(ys.reverse ::: xs1, path)
case _ :: xs1 =>
precedingPath(xs1, path)
case _ =>
path
}
precedingPath(roots.reverse, Nil)
}
}
17 changes: 17 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Comments.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import util.SourceFile
import util.Positions._
import util.CommentParsing._
import util.Property.Key
import util.Chars.{isIdentifierPart, isOperatorPart}
import parsing.Parsers.Parser
import reporting.diagnostic.messages.ProperDefinitionNotFound

// TODO Document. Also, this should not be in core. util or ast is better.
object Comments {
val ContextDoc = new Key[ContextDocstrings]

Expand All @@ -19,6 +21,8 @@ object Comments {
def docCtx: Option[ContextDocstrings] = ctx.property(ContextDoc)
}

private final val endCommentPrefix = "// end "

/** Context for Docstrings, contains basic functionality for getting
* docstrings via `Symbol` and expanding templates
*/
Expand Down Expand Up @@ -50,6 +54,19 @@ object Comments {

val isDocComment = raw.startsWith("/**")

/** If comment starts with `// end `, the identifier immediately following it.
* The identifier counts if the comment ends with it, or if it followed by
* a punctuation character in '.', ';', ','.
*/
val endCommentString: String =
if (raw.startsWith(endCommentPrefix)) {
val contents = raw.drop(endCommentPrefix.length)
val str = contents.takeWhile(c => isIdentifierPart(c) || isOperatorPart(c))
val follow = contents.drop(str.length)
if (follow.isEmpty || ".;,".contains(follow.head)) str else ""
}
else ""

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this add semantic meaning to comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, since you only get a warning if there's misalignment.

def expand(f: String => String): Comment = new Comment(pos, f(raw)) {
val isExpanded = true
val usecases = self.usecases
Expand Down
64 changes: 64 additions & 0 deletions compiler/src/dotty/tools/dotc/parsing/CheckAlignments.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package dotty.tools.dotc
package parsing

import ast.NavigateAST.precedingTrees
import core.Comments._
import core.Flags
import core.Decorators._
import core.Contexts._
import ast.{Positioned, Trees, untpd}
import util.SourceFile

object CheckAlignments {
import untpd._

private def kindString(t: Positioned)(implicit ctx: Context) = t match {
case t: ValDef => if (t.mods.is(Flags.Mutable)) "var" else "val"
case _: DefDef => "def"
case t: TypeDef => if (t.isClassDef) "class" else "type"
case _: ModuleDef => "object"
case _: PackageDef => "package"
case _: If => "if"
case _: Try => "try"
case _: Match => "match"
case _: WhileDo => "while"
case _: DoWhile => "do"
case _: ForDo | _: ForYield => "for"
case _ => ""
}

private def definedString(t: Positioned) = t match {
case t: MemberDef => t.name.toString
case _ => ""
}

def checkEndComments(source: SourceFile, endComments: List[Comment], roots: List[Tree])(implicit ctx: Context) = {
for (ec <- endComments) {
val endStr = ec.endCommentString
val column = source.column(ec.pos.start)
def misAligned(other: String): String =
i"misaligned '// end', corresponds to $other"
var warning: String = misAligned("nothing")
def combine(s1: String, s2: String) =
if (s1.isEmpty)
if (s2.isEmpty) "nothing" else s"'$s2'"
else
if (s2.isEmpty) s"'$s1'" else s"'$s1 $s2'"
def checkMatch(ts: List[Positioned]): Unit = ts match {
case t :: ts1 =>
if (source.column(t.pos.start) == column) {
if (endStr == kindString(t) || endStr == definedString(t))
warning = ""
else {
if (kindString(t).nonEmpty)
warning = misAligned(combine(kindString(t), definedString(t)))
checkMatch(ts1)
}
} else checkMatch(ts1)
case Nil =>
}
checkMatch(precedingTrees(ec.pos.start, roots))
if (warning.nonEmpty) ctx.warning(warning, ec.pos)
}
}
}
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import Comments._
import scala.annotation.{tailrec, switch}
import util.DotClass
import rewrite.Rewrites.patch
import CheckAlignments.checkEndComments

object Parsers {

Expand Down Expand Up @@ -960,6 +961,7 @@ object Parsers {
t
} else {
val t = expr()
newLineOptWhenFollowedBy(altToken)
accept(altToken)
t
}
Expand Down Expand Up @@ -2457,6 +2459,7 @@ object Parsers {
else
ts ++= topStatSeq()

checkEndComments(source, in.endComments, ts.toList)
ts.toList
}

Expand Down
22 changes: 11 additions & 11 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ object Scanners {
def nextPos: Int = (lookahead.getc(): @switch) match {
case ' ' | '\t' => nextPos
case CR | LF | FF =>
// if we encounter line delimitng whitespace we don't count it, since
// if we encounter line delimiting whitespace we don't count it, since
// it seems not to affect positions in source
nextPos - 1
case _ => lookahead.charOffset - 1
Expand All @@ -195,6 +195,11 @@ object Scanners {
/** Returns the closest docstring preceding the position supplied */
def getDocComment(pos: Int): Option[Comment] = docstringMap.get(pos)

private[this] var myEndComments = new ListBuffer[Comment]

/** Comments of the form `// end <ident>` */
def endComments: List[Comment] = myEndComments.toList

/** A buffer for comments */
val commentBuf = new StringBuilder

Expand Down Expand Up @@ -639,10 +644,8 @@ object Scanners {
}

private def skipComment(): Boolean = {
def appendToComment(ch: Char) =
if (keepComments) commentBuf.append(ch)
def nextChar() = {
appendToComment(ch)
commentBuf.append(ch)
Scanner.this.nextChar()
}
def skipLine(): Unit = {
Expand All @@ -667,14 +670,11 @@ object Scanners {
def nestedComment() = { nextChar(); skipComment() }
val start = lastCharOffset
def finishComment(): Boolean = {
if (keepComments) {
val pos = Position(start, charOffset - 1, start)
val comment = Comment(pos, flushBuf(commentBuf))
val pos = Position(start, charOffset - 1, start)
val comment = Comment(pos, flushBuf(commentBuf))

if (comment.isDocComment) {
addComment(comment)
}
}
if (keepComments && comment.isDocComment) addComment(comment)
if (comment.endCommentString.nonEmpty) myEndComments += comment

true
}
Expand Down
49 changes: 49 additions & 0 deletions tests/pos/withTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,58 @@ object Test with
.foldLeft(0) with
_ + _

// another test

xs.map with x =>
x + 2
.filter with x =>
x % 2 == 0
.foldLeft(0) with
_ + _

// for expressions

for
x <- List(1, 2, 3)
y <- List(x + 1)
yield
x + y

for
x <- List(1, 2, 3)
y <- List(x + 1)
do
println(x + y)

try
val x = 3
finally
println("done")

xs match
case Nil =>
println()
0
case x :: Nil =>
1
case _ => 2

do
println("x")
println("y")
while
println("z")
true

while
println("z")
true
do
println("x")
println("y")

// end while

// end Test

package p with
Expand All @@ -42,3 +87,7 @@ package p with
else
println("no")
false

// end C
// end object