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

ComputeImports: fix package computation. #7363

Merged
merged 1 commit into from May 14, 2024

Conversation

mbien
Copy link
Member

@mbien mbien commented May 7, 2024

attempt to fix #7073, #5537 and #6902

  • getPackageElement() requires the full package String or it returns null (e.g javax is not sufficient)
  • this caused a bug where the fix-imports-action imported members which matched root packages of random fully qualified declarations (e.g imports)
 // hit ctrl+shift+i, it shouldn't import anything
import java.util.List;
public class FixImports {
    public static class SomeClass2 {
        public static String java(String value) {
            return value;
        }
    }
}

@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels May 7, 2024
@mbien mbien requested a review from lahodaj May 7, 2024 03:33
@mbien mbien force-pushed the fix-imports-fix_delivery branch from 372b45b to 91178e7 Compare May 7, 2024 03:38
@mbien mbien linked an issue May 7, 2024 that may be closed by this pull request
@mbien mbien marked this pull request as ready for review May 8, 2024 01:29
@mbien mbien added this to the NB22 milestone May 8, 2024
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Minimal nitpick: The test sources seem to be missing package declarations.

getPackageElement() requires the full package or it returns null
@mbien mbien force-pushed the fix-imports-fix_delivery branch from 91178e7 to 56b02de Compare May 10, 2024 21:09
@mbien
Copy link
Member Author

mbien commented May 10, 2024

Minimal nitpick: The test sources seem to be missing package declarations.

@matthiasblaesing good that you mentioned this since I misunderstood how the test works. It uses now one test file since all it has to do is to check that the computed candidates are empty.

Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonably, thanks!

@ebarboni ebarboni merged commit 168c834 into apache:delivery May 14, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
6 participants