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 #14448: Updated IDEA to latest version v2023.3.4 #14604

Closed
wants to merge 1 commit into from

Conversation

MANISH-K-07
Copy link
Contributor

Aims to close #14448

Follow up of @nrmancuso 's checkstyle/contribution#837 (comment)

IDEA version used : v2023.3.4 (latest release)

Version: 2023.3.4
Build: 233.14475.28
13 February 2024

https://www.jetbrains.com/idea/download/?section=linux

Link to docker image used for testing update :
https://hub.docker.com/repository/docker/manishkk07/manish-k-07-checkstyle/tags?page=1&ordering=last_updated

To pull image : docker pull manishkk07/manish-k-07-checkstyle:jdk11-idea2023.3.4

Verified build on local....

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:38 h
[INFO] Finished at: 2024-03-04T19:55:19+05:30
[INFO] ------------------------------------------------------------------------

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 4, 2024

The failing CI checkstyle.checkstyle (Job spelling) because of my personal docker hub repo name

New misspellings found, please review:
manish
manishkk

patch config/jsoref-spellchecker/whitelist.words <<EOF
+++ .ci-temp/unknown.words	2024-03-04 14:51:12.742222543 +0000
 Mancuso
+manish
+manishkk

ci/circleci: run-inspections shows a successful build with inspection problems :

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  19.035 s
[INFO] Finished at: 2024-03-04T14:41:56Z
[INFO] ------------------------------------------------------------------------

####################################################################################################
Intellij Idea validation is about to start
Progress output will be flushed at end. Validation is in progress ...
Checking results ...

ATTENTION GITHUB CONTRIBUTOR:
There are inspection problems.
Review results in 'ARTIFACTS' tab in the CircleCI UI above, or follow the link below:
https://app.circleci.com/pipelines/github/checkstyle/checkstyle/1/workflows/30a26932-e1d7-4524-9059-cbcdaa98bdf3/jobs/538937/artifacts

Exited with code exit status 1

Should the inspection problems be dealt with in this PR itself ?

@nrmancuso
Copy link
Member

Should the inspection problems be dealt with in this PR itself ?

One thing at a time, let's get image update across the finish line in contribution repo.

However, we usually suppress each one to make CI green, then go back and remove suppressions and deal with the code.

@nrmancuso
Copy link
Member

@MANISH-K-07 please suppress by noinspection javadoc tag, you can search for a bunch of examples of this in the project. For noinspectionreason, we can create a new issue and do “until #….”

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 8, 2024

@MANISH-K-07 please suppress by noinspection javadoc tag, you can search for a bunch of examples of this in the project. For noinspectionreason, we can create a new issue and do “until #….”

Will do.. this is the first time I'm hearing about this tag, but I'm a quick learner ;)
It might take a bit of time coz there are many (and by many I mean atleast 100) suppressions which need to be searched and tags inserted manually in javadoc

Created Issue #14625

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 8, 2024

I have managed to ditch all inspection problems except :

  1. SuppressionAnnotation
  2. UNUSED_IMPORT

The @noinspection tag doesn't seem to work for the above two ?
Rest all I have added the suppressions and they work as expected !!
Am still trying to get a hold of the following classes to suppress (reason why I had disabled few at idea-inspections.xml) :

  • JavadocParser
<problem>
<file>file://$PROJECT_DIR$/target/generated-sources/antlr/com/puppycrawl/tools/checkstyle/grammar/javadoc/JavadocParser.java</file>
<line>8683</line>
<module>checkstyle</module>
<package>com.puppycrawl.tools.checkstyle.grammar.javadoc</package>
<entry_point TYPE="class" FQNAME="com.puppycrawl.tools.checkstyle.grammar.javadoc.JavadocParser.HeadTagEndContext"/>
<problem_class id="SuppressionAnnotation" severity="ERROR" attribute_key="ERRORS_ATTRIBUTES">Inspection suppression annotation</problem_class>
<description>Inspection suppression annotation <code>@SuppressWarnings("CheckReturnValue")</code> #loc</description>
<highlighted_element>@SuppressWarnings("CheckReturnValue")</highlighted_element>
<language>JAVA</language>
<offset>1</offset>
<length>37</length>
</problem>
  • JavaLanguageParserBaseVisitor
<problem>
<file>file://$PROJECT_DIR$/target/generated-sources/antlr/com/puppycrawl/tools/checkstyle/grammar/javadoc/JavadocParserBaseVisitor.java</file>
<line>13</line>
<module>checkstyle</module>
<package>com.puppycrawl.tools.checkstyle.grammar.javadoc</package>
<entry_point TYPE="class" FQNAME="com.puppycrawl.tools.checkstyle.grammar.javadoc.JavadocParserBaseVisitor"/>
<problem_class id="SuppressionAnnotation" severity="ERROR" attribute_key="ERRORS_ATTRIBUTES">Inspection suppression annotation</problem_class>
<description>Inspection suppression annotation <code>@SuppressWarnings("CheckReturnValue")</code> #loc</description>
<highlighted_element>@SuppressWarnings("CheckReturnValue")</highlighted_element>
<language>JAVA</language>
<offset>0</offset>
<length>37</length>
</problem>
  • JavadocParserVisitor
<problem>
<file>file://$PROJECT_DIR$/target/generated-sources/antlr/com/puppycrawl/tools/checkstyle/grammar/javadoc/JavadocParserVisitor.java</file>
<line>12</line>
<module>checkstyle</module>
<package>com.puppycrawl.tools.checkstyle.grammar.javadoc</package>
<entry_point TYPE="class" FQNAME="com.puppycrawl.tools.checkstyle.grammar.javadoc.JavadocParserVisitor"/>
<problem_class id="ClassWithTooManyDependencies" severity="ERROR" attribute_key="ERRORS_ATTRIBUTES">Class with too many dependencies</problem_class>
<hints/>
<description>Class 'JavadocParserVisitor' has too many dependencies (95 > 37)</description>
<highlighted_element/>
<language/>
</problem>
</problems>

EDIT : following ClassEscapesItsScope also survives with the javadoc annotation (disabled validation until further advice)

<problem>
<file>file://$PROJECT_DIR$/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java</file>
<line>256</line>
<module>checkstyle</module>
<package>com.puppycrawl.tools.checkstyle.checks.imports</package>
<entry_point TYPE="method" FQNAME="com.puppycrawl.tools.checkstyle.checks.imports.ImportControlLoader com.puppycrawl.tools.checkstyle.checks.imports.PkgImportControl load(java.net.URI uri)"/>
<problem_class id="ClassEscapesItsScope" severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Class is exposed outside of its visibility scope</problem_class>
<description>Class <code>PkgImportControl</code> is exposed outside its defined visibility scope #loc</description>
<highlighted_element>PkgImportControl</highlighted_element>
<language>JAVA</language>
<offset>18</offset>
<length>16</length>
</problem>
</problems>

@nrmancuso
Copy link
Member

We should not be checking generated files at all, please see if you can limit inspection scope to avoid these violations. For the ClassEscapesItsScope, what will it take to fix this? We could do this in a separate PR first, if it is something minimal.

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 13, 2024

We should not be checking generated files at all, please see if you can limit inspection scope to avoid these violations. For the ClassEscapesItsScope, what will it take to fix this? We could do this in a separate PR first, if it is something minimal.

@nrmancuso ,
One possibility I could see to avoid inspections running on generated files in target dir would involve execution changes in shell. I mean, we will have to update our working dir in concerned .sh file to our src dir to confine our scope.

PROJECT_DIR=$PWD/
INSPECTIONS_PATH=$PWD/config/intellij-idea-inspections.xml
RESULTS_DIR=$PWD/target/inspection-results

I believe changing PROJECT_DIR=$PWD/ --> PROJECT_DIR=$PWD/src will restrict inspections validations to our src folder.
Also, I think the same update needs to be done in batch file too ?

SET PROJECT_DIR=%CD%\

SET PROJECT_DIR=%CD%\ --> SET PROJECT_DIR=%CD%\src

Please confirm if this is what you meant by "please see if you can limit inspection scope to avoid these violations" so that I can proceed with making necessary code changes.
Or did you mean adding more limits at intellij-idea-inspection-scope.xml ?? 🤔

----------------------------------------------------------------------------------------------------

Coming to the ClassEscapesItsScope violation, I think it will be easier for us if we keep the validation disabled for now until migration gets through finish line, and later work on re-enabling it as part of separate issue/PR. Without migration, I'm not sure how it is possible to work on the violation in a separate PR because on a new branch (with older image), we won't even have this violation in the first place..

Please do correct me in case my understanding is wrong anywhere :)

@MANISH-K-07
Copy link
Contributor Author

@nrmancuso , ping please

@romani
Copy link
Member

romani commented Mar 15, 2024

Or did you mean adding more limits at intellij-idea-inspection-scope.xml ?? 🤔

yes

The @noinspection tag doesn't seem to work for the above two ?

lets disable them inn xml config and create issue to reactivate them, put comment above disablment until #xxxxxxxx.

@@ -2,5 +2,5 @@
all excludes should be specified in build configuration -->
<component name="DependencyValidationManager">
<scope name="Checkstyle Inspection Scope"
pattern="!file[checkstyle]:target//*&amp;&amp;!file:src/test/resources*//**&amp;&amp;!file:src/it/resources*//**&amp;&amp;!file:src/site/resources/js/google-analytics.js&amp;&amp;!file:src/site/resources/styleguides*//**&amp;&amp;!file:config/intellij-idea-inspections.properties&amp;&amp;!file[checkstyle]:.ci-temp//*&amp;&amp;!file[checkstyle]:bin//*&amp;&amp;!file[checkstyle]:.settings//*&amp;&amp;!file:.classpath&amp;&amp;!file:.project&amp;&amp;!file:.circleci/config.yml&amp;&amp;!file:config/projects-to-test/openjdk-projects-to-test-on.config&amp;&amp;!file:config/projects-for-no-exception-javadoc.config&amp;&amp;!file:.ci/pitest-survival-check-xml.groovy"/>
pattern="!file[checkstyle]:target//*&amp;&amp;!file:src/test/resources*//**&amp;&amp;!file:src/it/resources*//**&amp;&amp;!file:src/site/resources/js/google-analytics.js&amp;&amp;!file:src/site/resources/styleguides*//**&amp;&amp;!file:config/intellij-idea-inspections.properties&amp;&amp;!file[checkstyle]:.ci-temp//*&amp;&amp;!file[checkstyle]:bin//*&amp;&amp;!file[checkstyle]:.settings//*&amp;&amp;!file:.classpath&amp;&amp;!file:.project&amp;&amp;!file:.circleci/config.yml&amp;&amp;!file:config/projects-to-test/openjdk-projects-to-test-on.config&amp;&amp;!file:config/projects-for-no-exception-javadoc.config&amp;&amp;!file:.ci/pitest-survival-check-xml.groovy&amp;&amp;!file:target/generated-sources/antlr/com/puppycrawl/tools/checkstyle/grammar/java*//**"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@romani , @nrmancuso ,
Is what I did here wrong?
the multisegment wildcard that I added is theoretically supposed to match pattern path to exclude all files and
sub-dir of the pathspec.
But the violations in inspections continue to live on, meaning scope hasn't reduced..

Copy link
Member

@romani romani Mar 17, 2024

Choose a reason for hiding this comment

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

✔ ~/java/github/romani/checkstyle/target/generated-sources/antlr [master] 
$ ls -1
com
JavadocLexer.tokens
JavadocParser.tokens
JavaLanguageLexer.tokens
JavaLanguageParser.tokens

we already have pattern="!file[checkstyle]:target//* at the begining.
strange that it is not enough.
try (this is what IDEA generated when I excluded this folder)
&amp;&amp;*!file[checkstyle]:target/generated-sources/antlr//*
it is strange that it started to put such folder separtely:
image

Copy link
Member

@romani romani Mar 17, 2024

Choose a reason for hiding this comment

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

do you have the same problem on local ?
there is new IDEA release, there is somthing about inspections https://youtrack.jetbrains.com/articles/IDEA-A-2100661888/IntelliJ-IDEA-2023.3.5-233.14808.21-build-Release-Notes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we already have pattern="!file[checkstyle]:target//* at the begining.
strange that it is not enough.

Yes we do. I initially thought scope wasn't being restricted at /antlr because of the single-segment wildcard (only one level ls) that we used in our path pattern syntax. I already tried changing it to multi-segment (all levels below) but it was of no use. This is why I added precise pathspec which you see in the diff.

try (this is what IDEA generated when I excluded this folder)
&amp;&amp;*!file[checkstyle]:target/generated-sources/antlr//*

@romani , I'm still not sure if this will reach files of level //java because the pattern is still single-segment
https://cloud.google.com/eventarc/docs/path-patterns#path_pattern_syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you have the same problem on local ?

IntelliJ is known to be smart sometimes.. but this seems on an other level 😅
My tree looks fine on local with this scope pattern :

image

Idea version is 2020.3.1 (could be because of the older release)

there is new IDEA release, there is somthing about inspections https://youtrack.jetbrains.com/articles/IDEA-A-2100661888/IntelliJ-IDEA-2023.3.5-233.14808.21-build-Release-Notes

I couldn't find anything in their issue/pull that could explain what's happening here :(
That issue fixed in their release looks internal of IDEA, something related to it's error tab showing outdated errors
(shouldn't really bother us i think)

Copy link
Member

Choose a reason for hiding this comment

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

Have we tried to completely delete and regenerate (by UI) the scope file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have we tried to completely delete and regenerate (by UI) the scope file?

Yes @nrmancuso , I have completely removed the scope file from idea scopes and tried generating path tree by adding the pattern in the textField. It was still the same as #14604 (comment) on my local.

If it isn't much trouble, I would feel more confident about this if I get a confirmation from any of you before reporting this at JetBrains :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@romani , @nrmancuso , PR created at checkstyle/contribution#846 with v2023.1.2

Copy link
Member

Choose a reason for hiding this comment

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

please report issue to JetBrains.
Ideally old scope files should work (but not critical), main point is that scopes should work if they are setup by Idea UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@romani , Issue reported to jetBrains tracker at IDEA-351113

@MANISH-K-07
Copy link
Contributor Author

@romani , @nrmancuso , I will close this PR as migration is successful at #14696

The branch I will archive on my fork in case we might have to reuse later on....

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.

update version of IDEA inspection engine Version:2022.3.3
3 participants