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

Implement linter options #15503

Closed
odersky opened this issue Jun 22, 2022 · 8 comments
Closed

Implement linter options #15503

odersky opened this issue Jun 22, 2022 · 8 comments
Labels
itype:enhancement Semester Project Good project to be done by an MSc or strong Bsc computer science student in one semester stat:taken This project is already worked on by a student or as part of another volunteership program

Comments

@odersky
Copy link
Contributor

odersky commented Jun 22, 2022

Scala 3 is currently missing several of Scala 2's linter options, including:

  • unused variables
  • unused imports

It would be great to add these.

We could also look out for other valuable options to add.

@odersky odersky added itype:enhancement Semester Project Good project to be done by an MSc or strong Bsc computer science student in one semester labels Jun 22, 2022
@odersky
Copy link
Contributor Author

odersky commented Jun 22, 2022

If this project is taken as a semester project, we would have to synchronize with the state when the project starts. reserving the project as a semester project should not hold up progress that could be made before.

@griggt
Copy link
Contributor

griggt commented Jun 22, 2022

Note that there is already a PR for unused imports: #14524

@odersky
Copy link
Contributor Author

odersky commented Jun 22, 2022

Yes, @som-snytt what's the status on that? I notice it is still in draft state?

@griggt
Copy link
Contributor

griggt commented Jun 22, 2022

I can probably make some time to help with that one if needed.

@som-snytt
Copy link
Contributor

I'll rebase -Wunused:imports for further @griggt feedback. I don't think -rewrite option is a blocker, but it's important to handle language imports in a way that is friendly to cross-compilation. Thanks again, griggt. (Also I don't want to add to anyone's daily cognitive load if a feature isn't a priority.)

@odersky odersky added the stat:taken This project is already worked on by a student or as part of another volunteership program label Aug 14, 2022
@odersky
Copy link
Contributor Author

odersky commented Aug 14, 2022

Project reserved for Paul Cole.

@odersky
Copy link
Contributor Author

odersky commented Aug 20, 2022

See #15885 for another possible linter task.

szymon-rd added a commit that referenced this issue Jan 9, 2023
…d support for unused imports (#16157)

This PR, related to my **semester project** #15503 on adding **dotty's
linter features**, adds the following:

- [x] Add the `CheckUnused` front-end phase, which will check the the
tree produced by the typer, for unused entites (imports, local defs,
...)
- [x] Emit warning for `-Wunused:imports` including **given imports**
and **wildcard imports**
- [x] Emit warning for `-Wunused:locals`
- [x] Emit warning for `-Wunused:privates`
- [x] Emit warning for `-Wunused:params`
- ~~Emit warning for `-Wunused:patvars`~~
- [x] Emit warning for `-Wunused:unsafe-warn-patvars`
- [x] Emit warning for `-Wunused:linted`
- [x] Add a simple _fatal-warning_ compilation-test suit
- [x] _Fixes for the warning format_
- [x] Better help in CLI for `-Wunused`
- [x] Add `-Wunused:givens` alias to `-Wunused:implicits`

Here are a few examples:
#### Unused Imports
```scala
object Foo {
  import collection.mutable.{Set, Map}

  def main(args: Array[String]) =
    val bar = Set("this","set","is","used")
    println(s"Hello World: $bar")
}
``` 
```
sbt:scala3> scalac -Wunused:imports ../Foo.scala
[...]
-- Warning: ../scratch_files/Hello.scala:2:34 ----------------------------------
2 |  import collection.mutable.{Set, Map}
  |                                  ^^^
  |                                  unused import
1 warning found
```
#### Unused local definitions
```scala
class Foo {
  def bar =
    val a = 1
    2 + 2
}
``` 
```
sbt:scala3> scalac -Wunused:locals ../Foo.scala
[...]
-- Warning: ../scratch_files/MyHello.scala:3:8 ---------------------------------
3 |    val a = 1
  |    ^^^^^^^^^
  |    unused local definition
1 warning found
```
#### Unused private members
```scala
class Foo {
  private def a = 1
  private def b = 2

  def doSomething = b
}
``` 
```
sbt:scala3> scalac -Wunused:privates ../Foo.scala
[...]
-- Warning: ../scratch_files/MyHello.scala:2:14 --------------------------------
2 |  private def a = 1
  |  ^^^^^^^^^^^^^^^^^
  |  unused private member
1 warning found
```
#### Unused parameters
```scala
def foo(a: String)(using Int) = bar
``` 
```
sbt:scala3> scalac -Wunused:params ../scratch_files/Foo.scala
[...]
-- Warning: ../scratch_files/MyHello.scala:1:8 ---------------------------------
1 |def foo(a: String)(using Int) = bar
  |        ^
  |        unused explicit parameter
-- Warning: ../scratch_files/MyHello.scala:1:25 --------------------------------
1 |def foo(a: String)(using Int) = bar
  |                         ^
  |                         unused implicit parameter
2 warnings found
```
#### Unused pattern variables
```scala
def foo(a: List[Int]) = a match
    case head :: tail => ???
    case Nil => ???
``` 
```
sbt:scala3> scalac -Wunused:unsafe-warn-patvars ../scratch_files/Foo.scala
[...]
-- Warning: ../scratch_files/MyHello.scala:2:9 ---------------------------------
2 |    case head :: tail => ???
  |         ^^^^
  |         unused pattern variable
-- Warning: ../scratch_files/MyHello.scala:2:17 --------------------------------
2 |    case head :: tail => ???
  |                 ^^^^
  |                 unused pattern variable
2 warnings found
```

Please check the [test
file](tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala) for the
handled cases.

@odersky
@anatoliykmetyuk
szymon-rd added a commit that referenced this issue May 16, 2023
…7160)

This PR is related to my Bachelor Semester Project, supervised by
@anatoliykmetyuk.

The latter consist in improving and implementing more Scala 3 linter
options (see #15503), with #16639 as a starting issue fixed in this PR.

- During the traversal in CheckUnused.scala (Miniphase & local
TreeTraverser), when reaching an `Assign` case, symbols are collected as
set, and then used to filter used locals and privates variable at
reporting time.
- Adapt test suit, and Add more test accordingly.
- Note that for a same variable the unused warning always has priority
and shadows the unset warning.

That feature follows the Scala 2 `-Ywarn-unused:<args>` behavior.
@ckipp01
Copy link
Member

ckipp01 commented May 24, 2023

As of #16157 this is now implemented, so I'll go ahead and close! if there are any issues or specific follow-ups to this needed, please do create issues for them.

@ckipp01 ckipp01 closed this as completed May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
itype:enhancement Semester Project Good project to be done by an MSc or strong Bsc computer science student in one semester stat:taken This project is already worked on by a student or as part of another volunteership program
Projects
None yet
Development

No branches or pull requests

4 participants