Skip to content
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

Regression: false warning about non-used variable #11478

Open
DavidPerezIngeniero opened this issue Apr 10, 2019 · 13 comments
Open

Regression: false warning about non-used variable #11478

DavidPerezIngeniero opened this issue Apr 10, 2019 · 13 comments

Comments

@DavidPerezIngeniero
Copy link

DavidPerezIngeniero commented Apr 10, 2019

This code snippet compiles with no warning in Scala 2.12.4:

    describe("procesos") {
      it("incluye los procesos") { f =>
        val modeloProceso= ModeloRdsBuilder.modeloProceso("p")  // <=== Warning reported here
        f.insertaModelo(modeloProceso)
        val proceso = EntidadDependiente(modeloProceso.entity, Option(123), "proceso", Seq.empty, 1)
        val modelo = ModeloRdsBuilder.modelo().copy(procesos = Seq(proceso))
        val md5Icono = "abc123"
        val ep = f.setUp(
          modelo = modelo // <========= Variable "modelo" is used here!!!!!!!!!!!!!!!!
        )
        f.insertaIcono(proceso.icono.get, md5Icono)
        val result = f.consultaWeb(ep).get
        val procesos = result.processes

        assert(procesos.length == 1, procesos)
        val found = procesos.head
        assert(found.id == proceso.entity.id)
        assert(found.description == proceso.titulo)
        assert(
          found.href == DataSetEditorFormReference(
            f.areaId,
            f.dataSetId,
            EditorDataSetId.procesoEditor(ep, 1)
          )
        )
        assert(
          found.icon == Option(IconReference(AreaId.AdminArea, IconSource.Rds, md5Icono))
        )
      }

but with Scala versions 2.12.5, 2.12.6, 2.12.7, and 2.12.8 it complains in this way:

VistaConsultaWebSpec.scala:494:13: local val modelo in value $anonfun is never used
         val modelo = ModeloRdsBuilder.modelo().copy(procesos = Seq(proceso))

I can provide privately full sources if necessary.
We compile with SBT 1.1.2, Java 8 and Linux 64 bits.
As we compile with the flag to treat warnings as errors, we cannot adopt latest Scala versions.

@SethTisue
Copy link
Member

do you have a minimized, standalone reproducer?

@DavidPerezIngeniero
Copy link
Author

No, I don't have now the time to invest on it.

@SethTisue SethTisue added this to the Backlog milestone Apr 10, 2019
@SethTisue
Copy link
Member

@som-snytt does it look to you like something that might already be fixed in 2.13.x?

@som-snytt
Copy link

Is this ScalaTest? Is it a macro expansion? I don't know anything about ScalaTest.

On 2.12.5 there was scala/scala#6190 and follow-up scala/scala#6404 which had to do with for comprehensions, and similarly #11175 and #10287

I don't mind running a private build or looking at -Xprint:typer for this test. The other option is to turn off the warning for test.

@DavidPerezIngeniero
Copy link
Author

DavidPerezIngeniero commented Apr 11, 2019

That's right. It is a ScalaTest. We're using ScalaTest version 3.0.4.
Maybe some macro provided by ScalaTest isn't working well.

I'll try to upgrade to latest ScalaTest 3.0.5.

@DavidPerezIngeniero
Copy link
Author

DavidPerezIngeniero commented Apr 11, 2019

I've tested ScalaTest 3.0.5 and Scala 2.12.8 and I get the same warning.
Trying to modify slightly the source code in order to remove warning.

@som-snytt
Copy link

@DavidPerezIngeniero Thanks, I'll try to reproduce. I feel bad about changing source for a spurious warning. However, I like that ingeniero recalls the word ingenious.

@DavidPerezIngeniero
Copy link
Author

I attach the full source file.
VistaConsultaWebSpec.scala.txt

@som-snytt
Copy link

I made a reduction but not a reproduction. I see f.setUp is also called in "construye el formulario del filtro correctamente", followed by asserts. Here is sample output of -Xprint:typer for my simplified test:

    VistaSpec.this.describe("procesos")(VistaSpec.this.it.apply("incluye los procesos").apply(((f: VistaSpec.this.FixtureParam) => {
      val modelo: String = "";
      val ep: VistaSpec.this.EditorPrincipalCW = f.setUp(modelo, f.setUp$default$2, f.setUp$default$3, f.setUp$default$4, f.setUp$default$5, f.setUp$default$6, f.setUp$default$7, f.setUp$default$8, f.setUp$default$9, f.setUp$default$10, f.setUp$default$11, f.setUp$default$12, f.setUp$default$13);
      val result: Int = ep.length();
      {
        val $org_scalatest_assert_macro_expr: org.scalactic.Bool = {
          val $org_scalatest_assert_macro_left: result.type = result;
          val $org_scalatest_assert_macro_right: Int = 1;
          org.scalactic.Bool.binaryMacroBool($org_scalatest_assert_macro_left, "==", $org_scalatest_assert_macro_right, $org_scalatest_assert_macro_left.==($org_scalatest_assert_macro_right), scalactic.this.Prettifier.default)
        };
        VistaSpec.this.assertionsHelper.macroAssert($org_scalatest_assert_macro_expr, ep, scalactic.this.Prettifier.default, org.scalactic.source.Position.apply("VistaSpec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 29))
      }
    }))

Because modelo is the first parameter, the default args are invoked in order. With filtro also specified, it becomes a block, so maybe that explains the difference in behavior:

      val ep: VistaSpec.this.EditorPrincipalCW = {
        <artifact> val x$1: String = modelo;
        <artifact> val x$2: String = filtro;
        <artifact> val x$3: VistaSpec.this.GridRds = f.setUp$default$2;
        <artifact> val x$4: Boolean = f.setUp$default$4;
        <artifact> val x$5: Seq[VistaSpec.this.Vinculo] @scala.reflect.internal.annotations.uncheckedBounds = f.setUp$default$5;
        <artifact> val x$6: Seq[VistaSpec.this.Privilegio] @scala.reflect.internal.annotations.uncheckedBounds = f.setUp$default$6;
        <artifact> val x$7: Int = f.setUp$default$7;
        <artifact> val x$8: String = f.setUp$default$8;
        <artifact> val x$9: Boolean = f.setUp$default$9;
        <artifact> val x$10: Seq[VistaSpec.this.ReferenciaLibro] @scala.reflect.internal.annotations.uncheckedBounds = f.setUp$default$10;
        <artifact> val x$11: Seq[VistaSpec.this.ProcesoFormulario] @scala.reflect.internal.annotations.uncheckedBounds = f.setUp$default$11;
        <artifact> val x$12: Seq[VistaSpec.this.Datum] @scala.reflect.internal.annotations.uncheckedBounds = f.setUp$default$12;
        <artifact> val x$13: VistaSpec.this.OcultaBotones = f.setUp$default$13;
        f.setUp(x$1, x$3, x$2, x$4, x$5, x$6, x$7, x$8, x$9, x$10, x$11, x$12, x$13)
      };

If you post the snippet of output with scalacOptions += "-Xprint:typer"for "incluye los procesos", I can look further. You might also try changing the setup call to f.setUp(modelo=modelo, filtro=null), in case that just makes it work for you. Later, I'll try to make my example code complex enough to trigger the issue, but I may not be successful.

@DavidPerezIngeniero
Copy link
Author

Thanks for your work. I'll try shortly to provide more information.

@DavidPerezIngeniero
Copy link
Author

DavidPerezIngeniero commented Apr 23, 2019

Here is the requested data:

VistaConsultaWebSpec.txt

Using Scala 2.12.8.

@som-snytt
Copy link

Thanks. I spent some time with it, but unfortunately I see no clues. There are two previous calls to f.setUp(modelo); they are slightly different: one supplies a second arg, the other defines modelo outside the it block.

@DavidPerezIngeniero
Copy link
Author

DavidPerezIngeniero commented Apr 24, 2019

I'll ask the programmer who wrote the code to help me to find the clue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants