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

Review unused imports in classes in CDI 4 #600

Closed
hjohn opened this issue Apr 3, 2022 · 8 comments · Fixed by #731
Closed

Review unused imports in classes in CDI 4 #600

hjohn opened this issue Apr 3, 2022 · 8 comments · Fixed by #731
Milestone

Comments

@hjohn
Copy link

hjohn commented Apr 3, 2022

I'm seeing quite a lot of imports that are only used to refer to documentation while the class or interface itself is not dependent on such a class. Examples:

jakarta.enterprise.inject.Instance refers to jakarta.enterprise.util.AnnotationLiteral but only in @See

jakarta.enterprise.context.spi.Contextual refers to jakarta.enterprise.inject.CreationException only in docs

jakarta.enterprise.inject.spi refers to jakarta.enterprise.inject.Alternative and jakarta.enterprise.inject.Stereotype only in docs

jakarta.enterprise.inject.spi refers to jakarta.enterprise.context.Dependent, jakarta.enterprise.inject.Disposes, jakarta.enterprise.event.Observes and jakarta.enterprise.inject.Produces only in docs.

Is this intended? I personally consider this bad form and convert those to fully qualified names to avoid creating the illusion of dependencies on other parts of the system where there are none.

Note that if I remove those imports, a minimal set of classes (Contextual, CreationalContext, Instance, Annotated, Bean, BeanAttributes, InjectionPoint, TypeLiteral) could be extracted to support minimal implementations of this specification.

@Ladicek
Copy link
Contributor

Ladicek commented Apr 4, 2022

I do think this is intended, because it affects how the javadoc is rendered, but I haven't been here since the beginning, so it's just my guess :-)

@manovotn
Copy link
Contributor

manovotn commented Apr 4, 2022

I vaguely recall that this was discussed in meetings before CDI 2.0 and deliberately kept this way but OFC I am not 100% sure.

@Emily-Jiang
Copy link
Contributor

If the javadoc uses the full name, the import statement on the top can be omitted. It is better not to directly mention the import statement as it plays a part in the build.

@manovotn manovotn changed the title Unused imports in classes in CDI 4 Review unused imports in classes in CDI 4 Jul 25, 2022
@manovotn
Copy link
Contributor

Let's include this in the list of issues for future version, see #622
@hjohn if you'd like to send a PR, that would be awesome :)

@Ladicek
Copy link
Contributor

Ladicek commented Feb 28, 2023

I'd repurpose this to: we need to have a proper linter in the CDI project (and the TCK).

@starksm64
Copy link
Contributor

Checkstyle is what is commonly used, and the MicroProfile project has an example usage to review:
https://github.com/eclipse/microprofile-parent/blob/master/pom.xml

@manovotn
Copy link
Contributor

manovotn commented Nov 1, 2023

Checkstyle is what is commonly used, and the MicroProfile project has an example usage to review: https://github.com/eclipse/microprofile-parent/blob/master/pom.xml

I'd personally lean towards using formatter-maven-plugin along with impsort-maven-plugin as that does the formatting for any contributor the moment they run a build.
Obviously that also needs a set of rules which we'd have to take from somewhere or construct for ourselves.

@Emily-Jiang
Copy link
Contributor

+1 on creating a set of rules.

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 a pull request may close this issue.

5 participants