Skip to content

Fix possible NPE in maven ModuleInfoSelector#9181

Merged
ebarboni merged 1 commit intoapache:deliveryfrom
mbien:module-info-selector-npe_delivery
Feb 4, 2026
Merged

Fix possible NPE in maven ModuleInfoSelector#9181
ebarboni merged 1 commit intoapache:deliveryfrom
mbien:module-info-selector-npe_delivery

Conversation

@mbien
Copy link
Member

@mbien mbien commented Feb 2, 2026

SourceLevelQuery may return null. (e.g if the version string isn't valid)

code section was introduced a while ago #2037

fixes #9180

SourceLevelQuery may return null.
@mbien mbien added this to the NB29 milestone Feb 2, 2026
@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Maven [ci] enable "build tools" tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Feb 2, 2026
Copy link
Member

@neilcsmith-net neilcsmith-net left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks!

Aside - looking at uses of System.getProperty("java.version") suggests we might have quite a few things that could be removed in future.

@lahodaj
Copy link
Contributor

lahodaj commented Feb 3, 2026

I guess I wonder the "polarity" should be reversed, i.e. if there's no source level, then maybe we should consider the source level to be 9+ nowadays? Or does that have some known severe impact?

@mbien
Copy link
Member Author

mbien commented Feb 3, 2026

I guess I wonder the "polarity" should be reversed, i.e. if there's no source level, then maybe we should consider the source level to be 9+ nowadays? Or does that have some known severe impact?

like

if (sourceLevel == null || !sourceLevel.startsWith("1.")) { //NOI18N
    // both sourceLevel and ideJDK are 9+
    ret = hasModuleInfoCP.get();  
}

?

changing the bias should be possible I think. But I am not sure why the source level query exists in the first place, if there is a module-info file, it can't be <=8 anymore which seems to be the purpose of the check.

I would prefer to keep it as is for NB 29 and consider reverting #2037 for NB 30.

@neilcsmith-net
Copy link
Member

I had actually written the same thing as @lahodaj in my comment, and then decided to remove before posting for a similar reason that it's probably better to keep the code like this for NB29 and review more later.

@ebarboni
Copy link
Contributor

ebarboni commented Feb 4, 2026

will merge this as more works will be done in NB30

@ebarboni ebarboni merged commit d99e9fc into apache:delivery Feb 4, 2026
74 of 76 checks passed
@mbien mbien linked an issue Feb 5, 2026 that may be closed by this pull request
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) Maven [ci] enable "build tools" tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NPE Cannot invoke "String.startsWith(String)"

4 participants