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

Issue #14625: fix inspection violation OptionalGetWithoutIsPresent JavaParserTest testSingleLineComment #14811

Merged
merged 1 commit into from Apr 23, 2024

Conversation

MANISH-K-07
Copy link
Contributor

@MANISH-K-07 MANISH-K-07 commented Apr 19, 2024

Part of #14625

Deals with one problem of OptionalGetWithoutIsPresent inspection.

<problem>
<file>file://$PROJECT_DIR$/src/test/java/com/puppycrawl/tools/checkstyle/JavaParserTest.java</file>
<line>114</line>
<module>project</module>
<package>com.puppycrawl.tools.checkstyle</package>
<entry_point TYPE="method" FQNAME="com.puppycrawl.tools.checkstyle.JavaParserTest void testAppendHiddenSingleLineCommentNodes()"/>
<problem_class id="OptionalGetWithoutIsPresent" severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Optional.get() is called without isPresent() check</problem_class>
<description><code>Optional.get()</code> without 'isPresent()' check</description>
<highlighted_element>get</highlighted_element>
<language>JAVA</language>
<offset>52</offset>
<length>3</length>
</problem>

https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html
https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#isPresent--
https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#get--

https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#orElseThrow-java.util.function.Supplier-

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Apr 19, 2024

Green inspections ci....
https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25242/workflows/01a98bb4-58f8-453a-b5cd-2cf18309ce15/jobs/577755?invite=true#step-104-109_91

failing ci/circleci: javac21 due to some network response issue, irrelevant here I believe.
EDIT: error gone on rebase, obviously

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Apr 19, 2024

@romani ,
If the changes of this PR are approved, then I will proceed with other violations concerning this inspection.
(there are a lot more)

Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

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

Item:

.that(singleLineComment.isPresent())
.isTrue();

final DetailAST comment = singleLineComment.get();
Copy link
Member

Choose a reason for hiding this comment

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

Please just use Optional#orElseThrow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nrmancuso , orElseThrow requires a Supplier argument, only default constructor is compatible.
Without arg to Ctor, we get another inspection problem NewExceptionWithoutArguments

We have to set default to NullPointerException if we want to use this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nrmancuso nrmancuso merged commit d6d4a5d into checkstyle:master Apr 23, 2024
113 checks passed
@nrmancuso
Copy link
Member

nrmancuso commented Apr 23, 2024

@romani , If the changes of this PR are approved, then I will proceed with other violations concerning this inspection. (there are a lot more)

@MANISH-K-07 we will probably not be able to take a generic approach to this inspection unless all violations are in test classes. Any that are in test classes can be sent in one PR

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Apr 24, 2024

@nrmancuso , Please also see #14810 , #14814 , #14815 related to this issue

I have moved TailRecursion problems to new issue like you suggested :)

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.

None yet

2 participants