Skip to content

Exclude synthetic this.m, Any.m from import lookup #22695

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

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Mar 3, 2025

Follow-up to #22633 which restricts import prefixes to stable prefixes.

This commit further excludes this and super; these are arbitrary synthetic expressions introduced by macros.

Noticed at #22690 (comment) which now warns correctly.

Requires a unit test.

Fixes #22690

@som-snytt
Copy link
Contributor Author

The "fix" has an interaction with variable assignment. By chance, this.x = 42 would not record a reference.

How did that ever work? It gets an attachment on the way in. (So it wasn't by chance after all.)

  override def prepareForAssign(tree: Assign)(using Context): Context =
    tree.lhs.putAttachment(Ignore, ()) // don't take LHS reference as a read

Check for Ignore before refUsage.

@som-snytt som-snytt force-pushed the issue/22690-false-import-warn branch from 0739710 to fb1fe22 Compare March 3, 2025 16:05
@som-snytt som-snytt marked this pull request as ready for review March 3, 2025 21:34
@som-snytt
Copy link
Contributor Author

A simple macro doesn't emit this.eq with a synthetic position, so a unit test requires further "research".

It would be helpful to have "integration" tests that allowed sbt builds or scala-cli builds. That would simplify use of "tests with dependencies". In real terms, the CB tests already enforce that circe "just works". Maybe sbt build tests would be restricted to dependencies from the CBs.

@Gedochao Gedochao requested a review from sjrd March 4, 2025 08:23
@sjrd sjrd merged commit 63a02dd into scala:main Mar 4, 2025
29 checks passed
@som-snytt
Copy link
Contributor Author

I'll follow up with a unit test after reducing the circe example.

@som-snytt som-snytt deleted the issue/22690-false-import-warn branch March 4, 2025 15:01
@WojciechMazur WojciechMazur added this to the 3.7.0 milestone Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.7 regression - false negative warning: unused imports
3 participants