Fix #1286: Warn on inexistent imports that are not used. #1479
Conversation
|
Shouldn't this be an error instead of a warning? |
|
Given that they can be safely ignored without any semantic issue, it seemed that a warning was the reasonable choice. |
| case tree: Import => | ||
| val exprTpe = tree.expr.tpe | ||
| tree.selectors.foreach { | ||
| case ident @ Ident(name) if name != nme.WILDCARD && !exprTpe.member(name).exists => |
There was a problem hiding this comment.
You can factor our the common code in a helper function checkExists(name).
nicolasstucki
force-pushed
the
fix-#1286
branch
from
5d5bc61 to
c738655
Compare
nicolasstucki
force-pushed
the
fix-#1286
branch
2 times, most recently
from
5d5bc61 to
9d24628
Compare
|
All comments have been addressed. |
|
It is very handy for this check to differentiate between used and unused, thanks! But I agree with @smarter that it should be an error. |
| case Import(expr, selectors) => | ||
| val exprTpe = expr.tpe | ||
| def notExists(name: Name): Boolean = | ||
| name != nme.WILDCARD && !exprTpe.member(name).exists |
There was a problem hiding this comment.
I think you need to refine this so that you look for term names and for type names.
I also agree that it should be an error.
There was a problem hiding this comment.
Now it emits an error and it tests the name as a type name as well.
There was a problem hiding this comment.
I also made it encode the name. This was needed for example in scala.collections.immutable.::.
nicolasstucki
force-pushed
the
fix-#1286
branch
from
9d24628 to
36a3a8f
Compare
| @@ -4,6 +4,16 @@ object language { | |||
|
|
|||
| class Feature | |||
|
|
|||
| /** Enable scala.Dynamic */ | |||
There was a problem hiding this comment.
Can we leave everything in scala.language? My latest PR #1550 removed dotty.language completely.
Otherwise LGTM
There was a problem hiding this comment.
The change on #1550 will break this PR. I will have to find another way to handle the members in the language module.
There was a problem hiding this comment.
Yes, I would propose to have them all in scala.language, not dotty.language.
There was a problem hiding this comment.
Rebased on #1550 and made changes to find all language features in scala.language.
nicolasstucki
force-pushed
the
fix-#1286
branch
from
36a3a8f to
f97da2d
Compare
nicolasstucki
force-pushed
the
fix-#1286
branch
from
f97da2d to
0964f07
Compare
| val exprTpe = expr.tpe | ||
| def checkIdent(ident: Ident): Unit = { | ||
| val name = ident.name.asTermName.encode | ||
| def isDottyLanguageFeature = { // TODO Add new language features to scala.language |
There was a problem hiding this comment.
Why make an exception for language features? I would doubt we need one, but if we do, this needs to be explained in a comment.
|
Resolved that we need to solve the dotty language module issue (i.e. that it differs from scalac's) as part of this pull request. |
nicolasstucki
force-pushed
the
fix-#1286
branch
4 times, most recently
from
ac33252 to
1b52452
Compare
This commit also fixes scala#1583.
nicolasstucki
force-pushed
the
fix-#1286
branch
from
1b52452 to
a8136ff
Compare
|
Rebased. No longer depends on |
No description provided.