Skip to content

Keep imports of qualifier types and static fields in expandWildcardImports - #3114

Open
SulimanAbdulrazzaq wants to merge 1 commit into
diffplug:mainfrom
SulimanAbdulrazzaq:fix/expand-wildcards-qualified-names
Open

SulimanAbdulrazzaq wants to merge 1 commit into
diffplug:mainfrom
SulimanAbdulrazzaq:fix/expand-wildcards-qualified-names

Conversation

@SulimanAbdulrazzaq

Copy link
Copy Markdown

expandWildcardImports drops imports that the code still needs, so the output does not compile (#2833).

Two cases were missed:

  • A type used only to qualify a member, for example Collections.sort(values) or TimeUnit.SECONDS. JSON.toJSONString(...) from the issue is the same case. The visitor only collected ClassOrInterfaceType nodes, annotations and the declaring type of static methods. A qualifier like Collections is a NameExpr, and resolving the method sort gives only java.util.Collections.sort, which was matched against static wildcard imports. So import java.util.*; expanded to import java.util.List; and Collections lost its import.
  • Static fields and enum constants from a static wildcard import, for example PI from import static java.lang.Math.*; or SECONDS from import static java.util.concurrent.TimeUnit.*;. Only static method calls were collected, so import static java.lang.Math.*; expanded to import static java.lang.Math.max; and PI no longer compiled.

The visitor now also visits NameExpr:

  • a name that resolves to a static field or an enum constant is matched against the static wildcard imports;
  • a name that qualifies a method call, field access or method reference, and is not a variable, is resolved as a type and matched against the regular wildcard imports.

Names that do not resolve are skipped, so the first segment of a fully qualified name such as java.util.List.of() and local variables used as qualifiers change nothing.

Tests: two new cases in ExpandWildcardImportsStepTest, as resource files next to the existing JavaClassWithWildcards*.test ones. Once the test file is touched, the ratcheted forbidWildcardImports lint checks the whole file. The two existing inline cases (emptyClasspath, resolvesJdkXmlTypes) would then fail that lint, so their sources moved to resource files too, unchanged.

Before the fix, the new tests fail. For example, keepsTypesOnlyUsedAsQualifiers gets only import java.util.List;, and keepsStaticFieldsAndEnumConstants gets only import static java.lang.Math.max;. After the fix:

  • ./gradlew spotlessCheck passes;
  • ./gradlew :lib:build :testlib:test passes on Java 21 (482 tests, 0 failures);
  • the Java step tests pass on Java 17;
  • :plugin-gradle:test --tests JavaDefaultTargetTest passes.

Changelog entries are added to CHANGES.md, plugin-gradle/CHANGES.md and plugin-maven/CHANGES.md.

Fixes #2833

…ports

A type from a wildcard import that is only used to qualify a member,
such as Collections in Collections.sort(list) or TimeUnit in
TimeUnit.SECONDS, was never collected, so its import was dropped and
the result did not compile. The same happened to static fields and
enum constants brought in by a static wildcard import, such as PI
from import static java.lang.Math.*.

Resolve simple names: a static field or enum constant is matched
against static wildcard imports, and a name that qualifies a method
call, field access or method reference and is not a value is resolved
as a type and matched against the regular wildcard imports.

This branch has not been deployed

No deployments
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.

expandWildcardImports: necessary imports in method call expressions removed

1 participant