Fix missing warning for unused arguments (#960)

2024-11-22 22:10:15 +03:00 · 2020-07-02 13:39:42 +02:00 · 2020-07-02 13:39:42 +02:00 · 001509b3b8
commit 001509b3b8
parent b990f39784
17 changed files with 127 additions and 80 deletions
--- a/docs/CONTRIBUTING.md
+++ b/docs/CONTRIBUTING.md
@ -236,9 +236,6 @@ a vanilla GraalVM distribution. To run it, use:
 JAVA_HOME=<PATH_TO_GRAAL_HOME> ./enso.jar <CLI_ARGS>
 ```

-If you decide not to use the default launcher script, make sure to pass
-the `-XX:-UseJVMCIClassLoader` option to the `java` command.
-
 #### Installing the Jupyter kernel
 Enso has a highly experimental and not-actively-maintained Jupyer Kernel.
 To run it:
@ -411,7 +408,7 @@ Below are options uses by the Language Server:
 To run the Language Server on 127.0.0.1:8080 type:

 ```bash
-java -jar enso.jar \
+./enso.jar \
  --server \
  --root-id 3256d10d-45be-45b1-9ea4-7912ef4226b1 \
  --path /tmp/content-root
--- a/engine/runtime/src/main/scala/org/enso/compiler/core/IR.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/core/IR.scala
@ -350,14 +350,6 @@ object IR {

    override def children: List[IR] = imports ++ bindings

-    def transformExpressions(
-      fn: PartialFunction[Expression, Expression]
-    ): Module = {
-      copy(
-        bindings = bindings.map(_.mapExpressions(_.transformExpressions(fn)))
-      )
-    }
-
    override def toString: String =
      s"""
      |IR.Module(
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/ComplexType.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/ComplexType.scala
@ -266,7 +266,7 @@ case object ComplexType extends IRPass {

    val binding = Method.Binding(
      methodRef.duplicate(),
-      args,
+      args.map(_.duplicate()),
      body.duplicate(),
      location,
      passData.duplicate,
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/LambdaShorthandToLambda.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/LambdaShorthandToLambda.scala
@ -65,15 +65,12 @@ case object LambdaShorthandToLambda extends IRPass {
    ir: IR.Module,
    moduleContext: ModuleContext
  ): IR.Module =
-    ir.transformExpressions {
-      case x =>
-        x.mapExpressions(
-          runExpression(
-            _,
-            InlineContext(freshNameSupply = moduleContext.freshNameSupply)
-          )
-        )
-    }
+    ir.mapExpressions(
+      runExpression(
+        _,
+        InlineContext(freshNameSupply = moduleContext.freshNameSupply)
+      )
+    )

  /** Desugars underscore arguments to lambdas for an arbitrary expression.
    *
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/NestedPatternMatch.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/NestedPatternMatch.scala
@ -118,9 +118,7 @@ case object NestedPatternMatch extends IRPass {
      )
    )

-    ir.transformExpressions {
-      case x => desugarExpression(x, freshNameSupply)
-    }
+    ir.mapExpressions(desugarExpression(_, freshNameSupply))
  }

  /** Desugars nested pattern matches in an expression.
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/OperatorToFunction.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/OperatorToFunction.scala
@ -43,9 +43,7 @@ case object OperatorToFunction extends IRPass {
    ir: IR.Module,
    moduleContext: ModuleContext
  ): IR.Module =
-    ir.transformExpressions({
-      case x => runExpression(x, new InlineContext())
-    })
+    ir.mapExpressions(runExpression(_, new InlineContext()))

  /** Executes the conversion pass in an inline context.
    *
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/SectionsToBinOp.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/SectionsToBinOp.scala
@ -43,13 +43,13 @@ case object SectionsToBinOp extends IRPass {
  override def runModule(
    ir: IR.Module,
    moduleContext: ModuleContext
-  ): IR.Module = ir.transformExpressions {
-    case x =>
+  ): IR.Module =
+    ir.mapExpressions(
      runExpression(
-        x,
+        _,
        new InlineContext(freshNameSupply = moduleContext.freshNameSupply)
      )
-  }
+    )

  /** Performs section to binary operator conversion on an IR expression.
    *
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/lint/ShadowedPatternFields.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/lint/ShadowedPatternFields.scala
@ -56,9 +56,7 @@ case object ShadowedPatternFields extends IRPass {
    ir: IR.Module,
    @unused moduleContext: ModuleContext
  ): IR.Module = {
-    ir.transformExpressions {
-      case x => lintExpression(x)
-    }
+    ir.mapExpressions(lintExpression)
  }

  /** Lints for shadowed pattern fields.
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/lint/UnusedBindings.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/lint/UnusedBindings.scala
@ -47,9 +47,7 @@ case object UnusedBindings extends IRPass {
    ir: IR.Module,
    moduleContext: ModuleContext
  ): IR.Module =
-    ir.transformExpressions {
-      case x => x.mapExpressions(runExpression(_, InlineContext()))
-    }
+    ir.mapExpressions(runExpression(_, InlineContext()))

  /** Lints an arbitrary expression.
    *
@ -97,10 +95,9 @@ case object UnusedBindings extends IRPass {
    val isUsed = aliasInfo.graph.linksFor(aliasInfo.id).nonEmpty

    if (!isIgnored && !isUsed) {
-      binding.copy(
-        expression = runExpression(binding.expression, context)
-      )
-      binding.addDiagnostic(IR.Warning.Unused.Binding(binding.name))
+      binding
+        .copy(expression = runExpression(binding.expression, context))
+        .addDiagnostic(IR.Warning.Unused.Binding(binding.name))
    } else {
      binding.copy(
        expression = runExpression(binding.expression, context)
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/optimise/ApplicationSaturation.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/optimise/ApplicationSaturation.scala
@ -52,10 +52,9 @@ case object ApplicationSaturation extends IRPass {
    moduleContext: ModuleContext
  ): IR.Module = {
    val passConfig = moduleContext.passConfiguration
-    ir.transformExpressions({
-      case x =>
-        runExpression(x, new InlineContext(passConfiguration = passConfig))
-    })
+    ir.mapExpressions(
+      runExpression(_, new InlineContext(passConfiguration = passConfig))
+    )
  }

  /** Executes the analysis pass, marking functions with information about their
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/optimise/LambdaConsolidate.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/optimise/LambdaConsolidate.scala
@ -76,13 +76,12 @@ case object LambdaConsolidate extends IRPass {
    ir: IR.Module,
    moduleContext: ModuleContext
  ): IR.Module =
-    ir.transformExpressions {
-      case x =>
-        runExpression(
-          x,
-          new InlineContext(freshNameSupply = moduleContext.freshNameSupply)
-        )
-    }
+    ir.mapExpressions(
+      runExpression(
+        _,
+        new InlineContext(freshNameSupply = moduleContext.freshNameSupply)
+      )
+    )

  /** Performs lambda consolidation on an expression.
    *
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/optimise/UnreachableMatchBranches.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/optimise/UnreachableMatchBranches.scala
@ -69,9 +69,7 @@ case object UnreachableMatchBranches extends IRPass {
    ir: IR.Module,
    @unused moduleContext: ModuleContext
  ): IR.Module = {
-    ir.transformExpressions {
-      case x => optimizeExpression(x)
-    }
+    ir.mapExpressions(optimizeExpression)
  }

  /** Runs unreachable branch optimisation on an expression.
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/IgnoredBindings.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/IgnoredBindings.scala
@ -58,15 +58,12 @@ case object IgnoredBindings extends IRPass {
    ir: IR.Module,
    moduleContext: ModuleContext
  ): IR.Module =
-    ir.transformExpressions {
-      case x =>
-        x.mapExpressions(
-          runExpression(
-            _,
-            InlineContext(freshNameSupply = moduleContext.freshNameSupply)
-          )
-        )
-    }
+    ir.mapExpressions(
+      runExpression(
+        _,
+        InlineContext(freshNameSupply = moduleContext.freshNameSupply)
+      )
+    )

  /** Desugars ignored bindings for an arbitrary expression.
    *
@ -172,7 +169,8 @@ case object IgnoredBindings extends IRPass {
    }
  }

-  /** Generates a new argument name for `arg` if `isIgnored` is true.
+  /** Generates a new argument name for `arg` if `isIgnored` is true and updates
+    * all arguments metadata with their 'ignored' status.
    *
    * It also handles recursing through the default values.
    *
@ -187,6 +185,20 @@ case object IgnoredBindings extends IRPass {
    freshNameSupply: FreshNameSupply
  ): IR.DefinitionArgument = {
    arg match {
+      case spec @ IR.DefinitionArgument.Specified(
+            IR.Name.This(_, _, _),
+            _,
+            _,
+            _,
+            _,
+            _
+          ) =>
+        // Note [Ignored `this` Argument]
+        spec
+          .copy(defaultValue =
+            spec.defaultValue.map(resolveExpression(_, freshNameSupply))
+          )
+          .updateMetadata(this -->> State.Ignored)
      case spec: IR.DefinitionArgument.Specified =>
        if (isIgnored) {
          val newName = freshNameSupply
@ -342,3 +354,11 @@ case object IgnoredBindings extends IRPass {
    }
  }
 }
+
+/* Note [Ignored `this` Argument]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * `this` is implicitly added to all methods in the GenerateMethodBodies pass.
+ * It may however not be used in the method body and this should not emit an
+ * unused warning. So when processing function arguments, `this` should be
+ * marked as ignored to avoid warnings from the UnusedBindings pass.
+ */
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/TypeFunctions.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/TypeFunctions.scala
@ -53,9 +53,8 @@ case object TypeFunctions extends IRPass {
  override def runModule(
    ir: IR.Module,
    @unused moduleContext: ModuleContext
-  ): IR.Module = ir.transformExpressions {
-    case a => resolveExpression(a)
-  }
+  ): IR.Module =
+    ir.mapExpressions(resolveExpression)

  /** Performs typing function resolution on an expression.
    *
@ -68,9 +67,10 @@ case object TypeFunctions extends IRPass {
  override def runExpression(
    ir: IR.Expression,
    @unused inlineContext: InlineContext
-  ): IR.Expression = ir.transformExpressions {
-    case a => resolveExpression(a)
-  }
+  ): IR.Expression =
+    ir.transformExpressions {
+      case a => resolveExpression(a)
+    }

  // === Pass Internals =======================================================

--- a/engine/runtime/src/test/scala/org/enso/compiler/test/pass/analyse/GatherDiagnosticsTest.scala
+++ b/engine/runtime/src/test/scala/org/enso/compiler/test/pass/analyse/GatherDiagnosticsTest.scala
@ -117,8 +117,11 @@ class GatherDiagnosticsTest extends CompilerTest {
      val diagnostics = result
        .unsafeGetMetadata(GatherDiagnostics, "Impossible")
        .diagnostics
-      diagnostics should have length 1
-      diagnostics.head.message shouldEqual "Unused variable unused."
+      diagnostics should have size 2
+      diagnostics.map(_.message).toSet shouldEqual Set(
+        "Unused variable unused.",
+        "Unused function argument x."
+      )
    }
  }
 }
--- a/engine/runtime/src/test/scala/org/enso/compiler/test/pass/lint/UnusedBindingsTest.scala
+++ b/engine/runtime/src/test/scala/org/enso/compiler/test/pass/lint/UnusedBindingsTest.scala
@ -1,7 +1,7 @@
 package org.enso.compiler.test.pass.lint

 import org.enso.compiler.Passes
-import org.enso.compiler.context.{FreshNameSupply, InlineContext}
+import org.enso.compiler.context.{FreshNameSupply, InlineContext, ModuleContext}
 import org.enso.compiler.core.IR
 import org.enso.compiler.core.IR.Pattern
 import org.enso.compiler.pass.PassConfiguration._
@ -11,8 +11,9 @@ import org.enso.compiler.pass.optimise.ApplicationSaturation
 import org.enso.compiler.pass.{IRPass, PassConfiguration, PassManager}
 import org.enso.compiler.test.CompilerTest
 import org.enso.interpreter.runtime.scope.LocalScope
+import org.scalatest.Inside

-class UnusedBindingsTest extends CompilerTest {
+class UnusedBindingsTest extends CompilerTest with Inside {

  // === Test Setup ===========================================================

@ -57,6 +58,31 @@ class UnusedBindingsTest extends CompilerTest {
    )
  }

+  /** Adds an extension method for running linting on the input IR.
+    *
+    * @param ir the IR to lint
+    */
+  implicit class LintModule(ir: IR.Module) {
+
+    /** Runs unused name linting on [[ir]].
+      *
+      * @param moduleContext the inline context in which the desugaring takes
+      *                      place
+      * @return [[ir]], with all unused names linted
+      */
+    def lint(implicit moduleContext: ModuleContext): IR.Module = {
+      UnusedBindings.runModule(ir, moduleContext)
+    }
+  }
+
+  /** Makes a module context.
+    *
+    * @return a new inline context
+    */
+  def mkModuleContext: ModuleContext = {
+    ModuleContext(freshNameSupply = Some(new FreshNameSupply))
+  }
+
  // === The Tests ============================================================

  "Unused bindings linting" should {
@ -78,6 +104,31 @@ class UnusedBindingsTest extends CompilerTest {
      lintMeta.head.name.name shouldEqual "x"
    }

+    "attach a warning to an unused top-level function argument" in {
+      implicit val ctx: ModuleContext = mkModuleContext
+
+      val ir =
+        """
+          |f = x -> 10
+          |main =
+          |    f 0
+          |""".stripMargin.preprocessModule.lint
+
+      inside(ir.bindings.head) {
+        case definition: IR.Module.Scope.Definition.Method.Explicit =>
+          inside(definition.body) {
+            case f: IR.Function.Lambda =>
+              val lintMeta = f.arguments(1).diagnostics.collect {
+                case u: IR.Warning.Unused.FunctionArgument => u
+              }
+
+              lintMeta should not be empty
+              lintMeta.head shouldBe an[IR.Warning.Unused.FunctionArgument]
+              lintMeta.head.name.name shouldEqual "x"
+          }
+      }
+    }
+
    "not attach a warning to an unused function argument if it is an ignore" in {
      implicit val ctx: InlineContext = mkInlineContext

--- a/lib/searcher/src/test/scala/org/enso/searcher/sql/SuggestionsRepoTest.scala
+++ b/lib/searcher/src/test/scala/org/enso/searcher/sql/SuggestionsRepoTest.scala
@ -1,6 +1,6 @@
 package org.enso.searcher.sql

-import org.enso.jsonrpc.test.RetrySpec
+import org.enso.jsonrpc.test.FlakySpec
 import org.enso.searcher.Suggestion
 import org.scalatest.BeforeAndAfterAll
 import org.scalatest.matchers.should.Matchers
@ -13,7 +13,7 @@ import scala.concurrent.duration._

 class SuggestionsRepoTest
    extends AnyWordSpec
-    with RetrySpec
+    with FlakySpec
    with Matchers
    with BeforeAndAfterAll {

@ -35,7 +35,7 @@ class SuggestionsRepoTest

  "SuggestionsDBIO" should {

-    "select suggestion by id" in {
+    "select suggestion by id" taggedAs Flaky in {
      val action =
        for {
          id  <- db.run(repo.insert(suggestion.atom))
@ -45,7 +45,7 @@ class SuggestionsRepoTest
      Await.result(action, Timeout) shouldEqual Some(suggestion.atom)
    }

-    "find suggestion by returnType" taggedAs Retry() in {
+    "find suggestion by returnType" taggedAs Flaky in {
      val action =
        for {
          _   <- db.run(repo.insert(suggestion.local))