Add more documentation and examples, implement operator filter

jplag.frontend.scala/README.md

@@ -1,21 +1,92 @@
 # JPlag Scala language frontend
 
 The JPlag Scala frontend allows the use of JPlag with submissions in Scala. <br>
-It is based on the [Scalameta library](https://scalameta.org/) parser, and is adapted from the [CodeGra-de Scala frontend](https://github.com/CodeGra-de/jplag/tree/master/jplag.frontend.scala) for JPlag, both licensed under BSD-3.
+It is based on the [Scalameta library](https://scalameta.org/) parser, and is adapted from
+the [CodeGra-de Scala frontend](https://github.com/CodeGra-de/jplag/tree/master/jplag.frontend.scala) for JPlag, both
+licensed under BSD-3.
 
 ### Scala specification compatibility
 
-The dependencies only allow compatibility up to Scala 2.12 (April 2018), so more recent syntactical features like enums are not supported yet. 
+The dependencies only allow compatibility up to Scala 2.12 (April 2018), so more recent syntactical features like enums
+are not supported yet.
+
+Due to intercompatibility issues between dependencies, previous attempts to upgrade to Scala 3 were unsuccessful. We
+will continue our efforts to support Scala 3.
 
-Due to intercompatibility issues between dependencies, previous attempts to upgrade to Scala 3 were unsuccessful. We will continue our efforts to support Scala 3. 
- 
 ### Token Extraction
 
-The choice of tokens is intended to be similar to the Java or C# frontends. Specifically, among others, it includes a range of nesting structures (class and method declarations, control flow expressions) as well as variable declaration, object creation, assignment, and control flow altering keywords. <br>
-Blocks are distinguished by their context, i.e. there are separate `TokenConstants` for `if` blocks, `for` blocks, class bodies, method bodies, array constructors, and the like.
+#### General
+
+The choice of tokens is intended to be similar to the Java or C# frontends. Specifically, among others, it includes a
+range of nesting structures (class and method declarations, control flow expressions) as well as variable declaration,
+object creation, assignment, and control flow altering keywords. <br>
+Blocks are distinguished by their context, i.e. there are separate `TokenConstants` for `if` blocks, `for` blocks, class
+bodies, method bodies, array constructors, and the like.
+
+More syntactic elements of Scala may turn out to be helpful to include in the future, especially those that are newly
+introduced.
+
+#### Problem in Scala (1): Method calls
+
+The syntax of Scala allows to omit the parentheses when calling methods without arguments. These method calls are
+indistinguishable from member references. This makes the system vulnerable to attacks where an empty set of brackets is
+simply added after the member name. To address this, method calls with no arguments are not marked with an `APPLY`
+token, even though they are recognizable as method calls.
+
+```scala
+myObject.member // may be member reference or method call
+// gets no tokens
+
+myObject.member2() // must be method call
+// gets no tokens either!
+
+myObject.member3(arg1, arg2) // must be method call
+// gets APPLY ARG ARG tokens
+```
+
+#### Problem in Scala (2): Operators
+
+Operators are implemented as regular method calls. Additionally, custom operators on objects/classes can be defined,
+possibly overloading existing ones like `+`, `&=` etc.
+
+In other frontends, operations are not assigned tokens but "regular" method calls are. This calls for the task to try to
+distinguish operations from what we understand as "regular" method calls. This is not entirely possible with only
+parsing information, so we decided to go about this problem as follows:
+
+- Calls to methods with an identifier that is used as an operator are NOT treated as a method call. This is accomplished
+  by comparing to a hard-coded list of standard operators on numbers, booleans, lists, and types (although type
+  operators cannot be used in the same contexts as the others). This applies in infix and dot notation.
+- Calls to methods with any other identifier, be it alphanumerical, symbolic or any combination, are treated as method
+  calls and are assigned `APPLY` and `ARG` tokens if applicable, see (1).
+
+#### Problem in Scala (3): `return` is optional and discouraged
+
+In Scala, the use of the `return` keyword is regarded as a bad smell because it may disrupt the control flow in ways
+unintended by the less experienced Scala developer.
+Instead, like any block of code, the method body is evaluated to the last expression that is evaluated.
+
+```scala
+def power(base: Int, exponent: Int): Int = {
+    if (exponent == 0) 1                                // mark this return value?
+    else if (exponent == 1) base                        // and this one?
+    else if (exponent % 2 == 0) 
+        ((i: Int) => i*i)(power(base, exponent / 2))    // and this one?
+    else base * power(base, exponent - 1)               // and this one?
+}
+```
+That raises the question whether to try and mark these more implicit return values, so that the output of this frontend
+would be consistent with others.
+
+To determine all possible return values, semantic information about control structures is necessary which may be tedious
+to extract from the AST, but possible (e.g. by means of a stack mechanic).
+On the other hand, "the last expression of a block" does not hold the same _syntactical_ weight to it as a return
+statement.
 
-More syntactic elements of Scala may turn out to be helpful to include in the future, especially those that are newly introduced.
+For the moment, implicit block values are neglected.
 
 ### Usage
 
-To use the Scala frontend, add the `-l scala` flag in the CLI, or use a `JPlagOption` object set to `LanguageOption.SCALA` in the Java API as described in the usage information in the [readme of the main project](https://github.com/jplag/JPlag#usage) and [in the wiki](https://github.com/jplag/JPlag/wiki/1.-How-to-Use-JPlag).
+To use the Scala frontend, add the `-l scala` flag in the CLI, or use a `JPlagOption` object set
+to `LanguageOption.SCALA` in the Java API as described in the usage information in
+the [readme of the main project](https://github.com/jplag/JPlag#usage)
+and [in the wiki](https://github.com/jplag/JPlag/wiki/1.-How-to-Use-JPlag).

jplag.frontend.scala/src/main/scala/de/jplag/scala/Parser.scala

@@ -10,15 +10,36 @@ import scala.meta._
 class Parser extends AbstractParser {
     private var currentFile: String = _
 
-    var tokens: TokenList = _
+    private var tokens: TokenList = _
 
     private val traverser: Traverser = new Traverser {
 
+        private val Operators : Array[String] = Array(
+            "+", "-", "*", "/", "%", "**",
+            "==", "!=", ">", "<", ">=", "<=",
+            "&&", "||", "!",
+            "=", "+=", "-=", "*=", "/=", "%=", "**=", "<<=", ">>=", ">>>=", "&=", "^=", "|=",
+            "&", "|", "^", "<<", ">>", "~", ">>>",
+            "++", "::", ":::",
+            "<:", ">:", "#"
+        )
+
+        /**
+         * TraverserRecord; Identifier kept short for readability
+         * @param before opt. token type to insert at the beginning of the tree
+         * @param after opt. token type to insert at the end of the tree
+         * @param traverse custom method of traversing the tree
+         */
         case class TR(before: Option[ScalaTokenConstants.Value] = None,
                       after: Option[ScalaTokenConstants.Value] = None,
                       traverse: Tree => Unit = _.children.foreach(traverser.apply)
                      )
 
+        /**
+         * Adds a token at the beginning of the tree and traverses it if a tree is given.
+         * @param tree opt. tree
+         * @param token token type to insert at the beginning of the tree
+         */
         private def maybeAddAndApply(tree: Option[Tree], token: ScalaTokenConstants.Value): Unit = tree match {
             case Some(exp) =>
                 add(token, exp, fromEnd = false)
@@ -29,31 +50,35 @@ class Parser extends AbstractParser {
         private def processCases(cases: List[Case]): Unit = cases.foreach {
             case c@Case(pattern, condition, body) =>
                 add(ScalaTokenConstants.CaseStatement, c, fromEnd = false)
-                apply(pattern)
-                apply(condition)
-                enclose(body, TR(Some(CaseBegin), Some(CaseEnd)))
+                applyRecursively(Seq(pattern, condition))
+
+                encloseAndApply(body, TR(Some(CaseBegin), Some(CaseEnd)))
             case _ =>
         }
 
-        def isNotArithmetic(op: Term.Name): Boolean = {
-            op.value.matches("""[_\w]+""")
+        private def isStandardOperator(op: String): Boolean = {
+            Operators.contains(op)
+        }
+
+        private def getMethodIdentifier(fun: Term): String = {
+            fun.toString().split("\\.").last
         }
 
         private def doMatch(tree: Tree): TR = {
             tree match {
                 case Term.Do(body, condition) => TR(Some(DoWhile), Some(DoWhileEnd), traverse = _ => {
-                    enclose(body, TR(Some(DoBodyBegin), Some(DoBodyEnd)))
+                    encloseAndApply(body, TR(Some(DoBodyBegin), Some(DoBodyEnd)))
                     apply(condition)
                 })
-                case Term.Assign(_) => TR(Some(Assign), None)
+                case Term.Assign(_) => TR(Some(Assign))
                 case Term.While(condition, body) => TR(Some(While), traverse = _ => {
                     apply(condition)
-                    enclose(body, TR(Some(WhileBodyBegin), Some(WhileBodyEnd)))
+                    encloseAndApply(body, TR(Some(WhileBodyBegin), Some(WhileBodyEnd)))
                 })
                 case Term.For(enumerators, body) =>
                     TR(Some(For), traverse = _ => {
                         apply(enumerators)
-                        enclose(body, TR(Some(ForBodyBegin), Some(ForBodyEnd)))
+                        encloseAndApply(body, TR(Some(ForBodyBegin), Some(ForBodyEnd)))
                     })
                 case Term.Try(expr, catchExpr, finallyExpr) => TR(Some(TryBegin),
                     traverse = _ => {
@@ -73,14 +98,15 @@ class Parser extends AbstractParser {
                 case Term.TryWithHandler(expr, catchp, finallyp) => TR(Some(TryBegin),
                     traverse = _ => {
                         apply(expr)
-                        enclose(catchp, TR(Some(CatchBegin), Some(CatchEnd)))
+                        encloseAndApply(catchp, TR(Some(CatchBegin), Some(CatchEnd)))
 
                         maybeAddAndApply(finallyp, ScalaTokenConstants.Finally)
                     })
-                case Term.Apply(fun, args) =>
+                case Term.Apply(fun, args) if !isStandardOperator(getMethodIdentifier(fun)) =>
                     // `f()` can also be written as `f`, so we simply don't see `f()` as a function call
                     // But Java style function calls with no args should still be recognized
                     TR(traverse = _ => {
+
                         add(Apply, fun, fromEnd = false)
                         apply(fun)
                         for (arg <- args) {
@@ -107,13 +133,13 @@ class Parser extends AbstractParser {
                 case Term.ForYield(enumerators, body) => TR(traverse = _ => {
                     apply(enumerators)
                     add(ForBodyBegin, body, fromEnd = false)
-                    enclose(body, TR(Some(Yield), Some(ForBodyEnd)))
+                    encloseAndApply(body, TR(Some(Yield), Some(ForBodyEnd)))
                 })
                 case Term.If(conditionExpr, thenExpression, elseExpression) => TR(traverse = _ => {
                     add(If, tree, fromEnd = false)
                     apply(conditionExpr)
 
-                    enclose(thenExpression, TR(Some(IfBegin), Some(IfEnd)))
+                    encloseAndApply(thenExpression, TR(Some(IfBegin), Some(IfEnd)))
 
                     elseExpression match {
                         case Lit.Unit() => apply(elseExpression)
@@ -121,7 +147,7 @@ class Parser extends AbstractParser {
                             val elseStart = tree.pos.text.indexOf("else", thenExpression.pos.end - tree.pos.start)
                             val elsePosition = Position.Range(tree.pos.input, tree.pos.start + elseStart, tree.pos.start + elseStart + 4)
                             add(Else, elsePosition.startLine + 1, elsePosition.startColumn + 1, elsePosition.text.length)
-                            enclose(elseExpression, TR(Some(ElseBegin), Some(ElseEnd)))
+                            encloseAndApply(elseExpression, TR(Some(ElseBegin), Some(ElseEnd)))
                     }
                 })
 
@@ -132,17 +158,14 @@ class Parser extends AbstractParser {
                     TR(traverse = _ => {
                         applyRecursively(mods)
                         add(MethodDef, name, fromEnd = false)
-                        for (tParam <- typeParams) add(TypeParameter, tParam, fromEnd = false)
-                        for (params <- paramss)
-                            for (param <- params) add(Parameter, param, fromEnd = false)
+                        assignRecursively(typeParams, TypeParameter)
+                        assignRecursively(paramss, Parameter)
 
-                        enclose(body, TR(Some(MethodBegin), Some(MethodEnd)))
+                        encloseAndApply(body, TR(Some(MethodBegin), Some(MethodEnd)))
                     })
                 case Defn.Macro(mods, name, tparams, paramss, declType, body) => TR(Some(Macro), traverse = _ => {
-                    applyRecursively(mods)
-                    applyRecursively(tparams)
-                    applyRecursively(paramss)
-                    enclose(body, TR(Some(MacroBegin), Some(MacroEnd)))
+                    applyRecursively(Seq(mods, tparams, paramss))
+                    encloseAndApply(body, TR(Some(MacroBegin), Some(MacroEnd)))
                 })
                 case Defn.Class(_) =>
                     TR(Some(ClassBegin), Some(ClassEnd))
@@ -184,45 +207,39 @@ class Parser extends AbstractParser {
 
                 case Ctor.Secondary(_) =>
                     TR(Some(ConstructorBegin), Some(ConstructorEnd))
+
                 case Init(tpe, name, argss) if argss.nonEmpty => TR(traverse = _ => {
-                    for (args <- argss) {
-                        for (arg <- args) {
-                            add(Argument, arg, fromEnd = false)
-                            apply(arg)
-                        }
-                    }
+                    assignRecursively(argss, Argument, doApply = true)
                 })
                 case Enumerator.Guard(_) => TR(Some(Guard))
 
                 case Term.Param(_) => TR(traverse = _ => add(Parameter, tree, fromEnd = false))
                 case Term.NewAnonymous(_) => TR(Some(ClassBegin), Some(ClassEnd))
                 case Term.ApplyInfix(_, op, _, _) if op.value.contains("=") && !Array("==", "!=").contains(op.value) => TR(Some(Assign))
-                case Term.ApplyInfix(fun, op, typeArgs, args) if isNotArithmetic(op) => TR(traverse = _ => {
+                case Term.ApplyInfix(fun, op, typeArgs, args) if !isStandardOperator(op.value) => TR(traverse = _ => {
                     add(Apply, tree, fromEnd = false)
                     apply(fun)
-                    for (typeArg <- typeArgs) {
-                        add(TypeArgument, typeArg, fromEnd = false)
-                        apply(typeArg)
-                    }
-                    for (arg <- args) {
-                        add(Argument, arg, fromEnd = false)
-                        apply(arg)
-                    }
+                    assignRecursively(typeArgs, TypeArgument, doApply = true)
+                    assignRecursively(args, Argument, doApply = true)
                 })
                 case Term.Select(refObj, member) =>
                     TR(traverse = _ => {
                         apply(refObj)
-                        add(ScalaTokenConstants.Member, member, fromEnd = false)
+                        if (!isStandardOperator(member.value)) add(ScalaTokenConstants.Member, member, fromEnd = false)
                         apply(member)
                     })
                 case Term.ApplyType(_, typeArgs) => TR(traverse = _ => {
                     add(Apply, tree, fromEnd = false)
-                    for (typeArg <- typeArgs) add(TypeArgument, typeArg, fromEnd = false)
+                    assignRecursively(typeArgs, TypeArgument)
                 })
                 case Term.New(_) => TR(Some(NewObject))
                 case Self(_) => TR(Some(SelfType))
-                case block@Term.Block(_) if block.parent.nonEmpty && block.parent.get.isInstanceOf[Term.Apply] =>
-                    TR(Some(BlockStart), Some(BlockEnd))
+                case block@Term.Block(_) => block.parent match {
+                        // inner block
+                        case Some(Term.Apply(_)) => TR(Some(BlockStart), Some(BlockEnd))
+                        // block in an expression context, e.g. for, if, while
+                        case _ => TR()
+                    }
                 case Enumerator.Generator(_) => TR(Some(EnumGenerator))
                 case meta.Type.Param(_) => TR(Some(TypeParameter))
 
@@ -230,10 +247,16 @@ class Parser extends AbstractParser {
             }
         }
 
-        def applyRecursively[T](els: List[T]): Unit = els.foreach({
+        /**
+         * Traverses a sequence of elements, which may in turn be lists as well.
+         * @param els a sequence of values, Trees or Lists
+         * @tparam T the type of elements
+         */
+        def applyRecursively[T](els: Seq[T]): Unit = els.foreach {
             case tree: Tree => apply(tree)
             case treeList: List[_] => applyRecursively(treeList)
-        })
+            case _ =>
+        }
 
         override def apply(tree: Tree): Unit = {
             val record: TR = doMatch(tree)
@@ -251,7 +274,28 @@ class Parser extends AbstractParser {
             }
         }
 
-        def enclose(tree: Tree, record: TR): Unit = {
+        /**
+         * Assigns the same token type to each element of a list of e.g. parameters.
+         * @param els list of equal elements
+         * @param tokenType type of token to assign to each of the elements
+         *                   @param doApply if true, the substructure of each element is traversed
+         * @tparam T list may contain more lists or leaves
+         */
+        def assignRecursively[T](els: List[T], tokenType: ScalaTokenConstants.Value, doApply: Boolean = false): Unit = els.foreach {
+            case treeList: List[_] => assignRecursively(treeList, tokenType)
+            case el: Tree =>
+                add(tokenType, el, fromEnd = false)
+                if (doApply) apply(el)
+        }
+
+        /**
+         * Adds a token to the beginning of the tree, traverses the tree, adds a token at the end of the tree.
+         * The token types are given by record.
+         *
+         * @param tree Tree to enclose and traverse
+         * @param record contains token types
+         */
+        def encloseAndApply(tree: Tree, record: TR): Unit = {
             record.before match {
                 case Some(value) => add(value, tree, fromEnd = false)
                 case None =>
@@ -283,7 +327,7 @@ class Parser extends AbstractParser {
         tokens
     }
 
-    def parseFile(dir: File, file: String): Boolean = {
+    private def parseFile(dir: File, file: String): Boolean = {
         currentFile = file
 
         try {
@@ -304,11 +348,24 @@ class Parser extends AbstractParser {
         true
     }
 
+    /**
+     * Adds a token to the token list.
+     * @param tType the type of the token
+     * @param line line of the occurrence in the file
+     * @param column column of the occurrence in the file
+     * @param length length of the occurrence in the file
+     */
     private def add(tType: ScalaTokenConstants.Value, line: Int, column: Int, length: Int): Unit = {
         tokens.addToken(new ScalaToken(tType.id, currentFile, line, column, length))
     }
 
 
+    /**
+     * Adds a token to the token list.
+     * @param tType the type of the token
+     * @param node the tree that marks the occurrence
+     * @param fromEnd if true, the token is added at the end of the tree (length 0).
+     */
     private def add(tType: ScalaTokenConstants.Value, node: Tree, fromEnd: Boolean): Unit = {
         if (node.pos.text.nonEmpty) {
             // SELF type tokens with no text content mess up the sequence

jplag.frontend.scala/src/test/resources/de/jplag/scala/Complete.scala

@@ -109,9 +109,43 @@ class Complete extends SuperClass with MyTrait {
         }
         println("b is not accessible here.")
     }
+
+    def operators(): Unit = {
+        // This is not treated as a function call
+        val a = 1 + 2
+        // This is treated as a function call
+        val b = (1).+(2)
+    }
+
+    def functionObject(): Unit = {
+        var f : Int => Int = _ * 2
+        f(2)
+
+        ((i: Int) => i * 2)(2)
+    }
+
+    def memberChain(): Unit = {
+        a.b.c
+    }
+
+    def infix() : Unit = {
+        val list = List(1,2,3)
+        val list2 = list reverse_::: list
+    }
+
+    def power(base: Int, exponent: Int): Int = {
+        if (exponent == 0) 1
+        else if (exponent == 1) base
+        else if (exponent % 2 == 0) {
+            ((i: Int) => i * i) (power(base, exponent / 2))
+        }
+        else base * power(base, exponent - 1)
+    }
+
     /*
-    Enums are a feature of Scala3, not supported by the parser.
+    Enums are a feature of Scala3, not yet supported by the parser.
     enum Menu:
         case PizzaMargharita, SpaghettiBolognese, Fries
     */
+
 }