scalameta / scalagen

WIP - Scalameta powered code generation
Apache License 2.0
40 stars 5 forks source link

SBT plugin will most likely fail compile when treating errors as warnings #42

Open DavidDudson opened 6 years ago

DavidDudson commented 6 years ago

Currently, we do not touch imports in generated files.

When we expand a generator, we remove the annotation.

Thus, code such as

   import foo.bar.Main

   @Main object App {
        println("Foo")
    }

will remove the annotation, leaving the import unused.

The solution to this problem is likely hard, but it should be noted somewhere in the docs at the least.

tabdulradi commented 6 years ago

Is it possible to configure SBT to treat the generated files differently? (i.e different scalac flags)

DavidDudson commented 6 years ago

No, because then it masks real errors. With a lot of the macro annotations I have seen, the generated code makes up a small portion of the file. You still want compiler warnings for the parts which are untouched.

It would be nice if we could ignore scalac warnings for ranges in documents. I think the eventual approach will be to remove select unused imports though.

olafurpg commented 6 years ago

The original and expanded code should be in separate projects where you can tune the scalacOptions and logger level via sbt. In the generated sources you may want to filter out warnings which can be done with a

      logLevel := Level.Error

You can also override the sbt compilerReporter where you get positions of reported messages, but I don't think that's necessary.

DavidDudson commented 6 years ago

The problem I see is something like

import scala.meta.generators.kase
import cats.NonEmptyList

@kase class Point(x: Int, y: Int)

In the above case:

  1. NonEmptyList is unused
  2. When treating warnings as errors with unused imports enabled it should fail
  3. Otherwise, it should emit a single warning.

After some thought, here's a few ideas I have.

Remove unused imports before compile

Remove all unused imports in file. This would cause compiling when treating warnings as errors to pass.

Cons:

Selectively remove unused imports only when compiler flag is detected

Remove any unused import of a Generator Subtype.

We only have to do this when detecting the unused imports flag, otherwise, we do not care.

Cons:

Just do not remove annotations

The only reason we actually get this error is due to removing the annotation, thus leaving the import unused.

Cons:

Force generator authors to use fully qualified names with unused imports

import cats.NonEmptyList

@scala.meta.generators.kase class Point(x: Int, y: Int)

This becomes a non-issue and we do not have to change anything.

Cons:

Note: After some thought, we do not know where the import is, thus we cannot just ignore warnings for this. Even a compiler plugin would not be able to figure it out. So ranges do not help.

To be truly honest, I think this might be the last nail in the coffin for recursion because I like that idea best. Perhaps we should ask the community.

@olafurpg Do you agree?

aparo commented 6 years ago

IMHO, the best solutions is to not remove the annotations. The generator library should not be required to be in the classpath only the annotation declaration is required. About the recursion, we can mark in the cache which modification is executed so we have an order of already executed ones.

DavidDudson commented 6 years ago

@apari that is the final solution at this stage... nearly complete. We are removing recursion, although you will be able to call other generators from yours, so you an do the same thing as recursion, just without the “discovery” phase.