Kill jsr-305 #4604

jtnord · 2020-03-26T19:10:54Z

remove JSR-305 annotations from core.

See JENKINS-55973.

Proposed changelog entries

Developer: Switched bug detection annotations from JSR-305 to SpotBugs / net.jcp equivalents

Proposed upgrade guidelines

N/A

Submitter checklist

JIRA issue is well described
Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
- Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
Appropriate autotests or explanation to why this change has no tests
For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

There are at least 2 approvals for the pull request and no outstanding requests for change
Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
Changelog entries in the PR title and/or Proposed changelog entries are correct
Proper changelog labels are set so that the changelog can be generated automatically
If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
If it would make sense to backport the change to LTS, a JIRA issue should exist and be labeled as lts-candidate

…heckReturnValue/g

…gs.annotations.OverrideMustInvoke/g

…ForNull/g

jtnord · 2020-03-26T19:13:06Z

Not sure how we can prevent someone adding an annotation back in (esp when the jsr-305 annotations are used in dependencies (and we still would like spotbugs to detect issues with the use of those libraries...)

timja · 2020-03-26T19:29:25Z

Not sure how we can prevent someone adding an annotation back in (esp when the jsr-305 annotations are used in dependencies (and we still would like spotbugs to detect issues with the use of those libraries...)

checkstyle can do it https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/imports/IllegalImportCheck.html

timja · 2020-03-26T19:50:27Z

Are you aware of: google/guava#2960 (comment) ?

jtnord · 2020-03-27T09:47:04Z

checkstyle can do it https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/imports/IllegalImportCheck.html

I was tying to avoid checkstyle - as that sounds from the outset like it would be more contentious to introduce - even with a single rule.. But I'll give it whirl and see what the feedback is

Are you aware of: google/guava#2960 (comment) ?

I am now :-) I think there is value in this PR as-is even if here could be another migration when that project comes to light and the tooling is updated to support it. (because we would be migrating from two sets of annotations not three)

jtnord · 2020-03-27T11:09:00Z

@timja added checkstyle to prevent re-introduction.

timja

I've done a cursory look through, but the diff is huge, one comment on checkstyle but non-blocking

timja · 2020-03-27T11:28:41Z

pom.xml

+          <configuration>
+             <consoleOutput>true</consoleOutput>
+             <failsOnError>true</failsOnError>
+             <!-- define here because checkstyle and multi module is a PITA -->


would be nice to have this in a file,
here's an example of how we got it working in multi module with jcasc:
https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/pom.xml#L82

non-blocking though

Thanks for the pointer.
Given I have no idea if tools (eclipse intellij netbeans etc) all cope with that correctly (given it is set by the mvn wrapper) - and not everyone imports all the projects in a multi module setup, I'll stick which this for now as I know it to work.

Justification for rejecting this feedback seems weak. Done in #6552.

jglick · 2020-03-30T19:23:04Z

Not sure how we can prevent someone adding an annotation back in

Add to bom a dependencyManagement entry pinning that artifact at a visibly bogus version. Which is probably how we should be handling javax.servlet:servlet-api, rather than using the [0] trick.

jglick

For the benefit of NetBeans users, please

diff --git core/src/main/resources/META-INF/upgrade/JSR-305.hint core/src/main/resources/META-INF/upgrade/JSR-305.hint
new file mode 100644
index 0000000000..14c9a4227c
--- /dev/null
+++ core/src/main/resources/META-INF/upgrade/JSR-305.hint
@@ -0,0 +1,3 @@
+javax.annotation.CheckForNull => edu.umd.cs.findbugs.annotations.CheckForNull;;
+javax.annotation.Nonnull => edu.umd.cs.findbugs.annotations.NonNull;;
+javax.annotation.Nullable => edu.umd.cs.findbugs.annotations.Nullable;;

jglick · 2020-03-30T19:26:23Z

war/pom.xml

@@ -1,4 +1,3 @@
-<?xml version="1.0" encoding="UTF-8"?>


Why? Please revert.

lord knows.. I edited this file via eclipse dependency panel and not by hand.

jeffret-b

I'm in favor of this cleanup. I also did a cursory review of some of the files. Everything I saw looked good. I didn't review everything though.

jtnord · 2020-03-30T22:04:53Z

Not sure how we can prevent someone adding an annotation back in

Add to bom a dependencyManagement entry pinning that artifact at a visibly bogus version. Which is probably how we should be handling javax.servlet:servlet-api, rather than using the [0] trick.

handled by checkstyle at the import level. want to keep them available for third party libs that use them.

Refer to jenkinsci/jenkins#4604 and https://issues.jenkins-ci.org/browse/JENKINS-55973 for the rationale. Replaces the deprecated javax.annotations references with non-deprecated references to the spotbugs annotations package. The deprecated javax.annotations were removed from Jenkins 2.231 and later.

…#4604

The parent pom provides spotbugs reports by default. Let's use spotbugs annotations because they are used in other Jenkins plugins and in Jenkins core. See https://issues.jenkins.io/browse/JENKINS-55973 and jenkinsci/jenkins#4604

Replace JSR305 annotations with SpotBugs annotations As of jenkinsci/jenkins#4604, core switched from JSR 305 to SpotBugs annotations. As of jenkinsci/plugin-pom#467 the plugin parent POM no longer delivers JSR 305. Therefore, we should make sure that we remove our reliance on this to prevent breakages for folks in the future.

Details at jenkinsci/jenkins#4604 See also https://issues.jenkins.io/browse/JENKINS-55973 https://docs.google.com/document/d/1PKYIpPlRVGsBqrz0Ob1Cv3cefOZ5j2xtGZdWs27kLuw/edit#heading=h.vxl6or38wd08

The parent pom provides spotbugs reports by default. Let's use spotbugs annotations because they are used in other Jenkins plugins and in Jenkins core. See https://issues.jenkins.io/browse/JENKINS-55973 and jenkinsci/jenkins#4604

* Show user docs on plugins.jenkins.io The current documentation link on plugins.jenkins.io is broken https://plugins.jenkins.io/basic-branch-build-strategies/ This change will fix that broken link and replace it with the user documentation the next time the plugin is released. The user documentation is quite good and will help users much better than the broken link from the plugins page. * Use spotbugs annotations instead of javax annotations The parent pom provides spotbugs reports by default. Let's use spotbugs annotations because they are used in other Jenkins plugins and in Jenkins core. See https://issues.jenkins.io/browse/JENKINS-55973 and jenkinsci/jenkins#4604 * Use Hamcrest assertThat, not JUnit deprecated assertThat https://junit.org/junit4/javadoc/4.13/org/junit/Assert.html#assertThat(java.lang.String,%20T,%20org.hamcrest.Matcher) * Remove unused imports * Remove two more unused imports * Cleanup and migrate docs to github * Address review feedback * Fix broken link to contributing doc * Address typo in CHANGES.adoc Co-authored-by: res0nance <[email protected]> Co-authored-by: Stargator <[email protected]>

jtnord added 9 commits March 26, 2020 11:10

s/javax.annotation.CheckReturnValue/edu.umd.cs.findbugs.annotations.C…

5abd08c

…heckReturnValue/g

s/javax.annotation.Nullable/edu.umd.cs.findbugs.annotations.Nullable/g

a7fcc8e

s/javax.annotation.OverridingMethodsMustInvokeSuper/edu.umd.cs.findbu…

928d40f

…gs.annotations.OverrideMustInvoke/g

s/javax.annotation.Nonnull/edu.umd.cs.findbugs.annotations.NonNull/g

819f80b

remove invalid NonNull annotation on a ctor

05e2b23

s/javax.annotation.CheckForNull/edu.umd.cs.findbugs.annotations.Check…

1f94696

…ForNull/g

s/javax.annotation.concurrent.GuardedBy/net.jcip.annotations.GuardedBy/

6b5c81d

s/javax.annotation.concurrent.Immutable/net.jcip.annotations.Immutable/

ba30c4b

remove Nonnegative annotation from JSR-305

5674de1

jtnord added the developer Changes which impact plugin developers label Mar 26, 2020

jtnord requested a review from a team March 26, 2020 19:11

add checkstyle rule to prevent re-introduction of jsr-305

4d3e60a

jtnord marked this pull request as ready for review March 27, 2020 11:09

timja approved these changes Mar 27, 2020

View reviewed changes

This was referenced Mar 27, 2020

Replace javax annotations with spotbugs annotations jenkinsci/git-plugin#859

Merged

Replace javax annotations with spotbugs annotations jenkinsci/git-client-plugin#529

Merged

jtnord added 3 commits March 30, 2020 12:40

Merge remote-tracking branch 'origin/master' into kill-jsr-305

bdcb2fb

don't bundled jsr-305 in the war

4de2344

ignore .checkstyle files

f7f07b8

jtnord requested review from a team March 30, 2020 18:47

jglick reviewed Mar 30, 2020

View reviewed changes

jeffret-b approved these changes Mar 30, 2020

View reviewed changes

jglick mentioned this pull request Apr 15, 2020

Remove JSR-305 jenkinsci/docker-fixtures#27

Merged

MarkEWaite mentioned this pull request May 4, 2020

Use NonNull annotation instead of deprecated Nonnull jenkins-infra/jenkins.io#3154

Merged

This was referenced May 10, 2020

Add javax.annotation as a dependency jenkinsci/jenkinsfile-runner#231

Closed

Replace jsr-305 annotations by SpotBugs ones jenkinsci/jenkinsfile-runner#296

Closed

devkonarev mentioned this pull request May 13, 2020

Replace jsr-305 annotations with SpotBugs ones jenkinsci/jenkinsfile-runner#298

Merged

basil added a commit to basil/swarm-plugin that referenced this pull request May 29, 2020

Remove JSR-305 annotations following the example in jenkinsci/jenkins…

b02e7b8

…#4604

jglick mentioned this pull request Jul 8, 2020

Code cleanup jenkinsci/workflow-multibranch-plugin#89

Merged

StefanSpieker mentioned this pull request May 14, 2021

Remove JSR-305 annotations from Blue-Ocean and replaced it with Spotbugs. jenkinsci/blueocean-plugin#2169

Merged

7 tasks

offa mentioned this pull request Jun 17, 2021

Replace JSR-305 annotations jenkinsci/pipeline-utility-steps-plugin#103

Merged

6 tasks

offa mentioned this pull request Aug 12, 2021

Code cleanups jenkinsci/workflow-basic-steps-plugin#151

Merged

6 tasks

offa mentioned this pull request Aug 20, 2021

Code cleanup / remove JSR 305 jenkinsci/remote-file-plugin#41

Merged

6 tasks

offa mentioned this pull request Oct 13, 2021

Missing nullability annotations added and JSR 305 replaced jenkinsci/ssh-agents-plugin#260

Merged

2 tasks

StefanSpieker mentioned this pull request Oct 15, 2021

Replace JSR 305 annotations with spotbugs annotations jenkinsci/code-coverage-api-plugin#233

Merged

6 tasks

StefanSpieker mentioned this pull request Oct 22, 2021

Replaced jsr-305 annotations with spotbugs annotations jenkinsci/kubernetes-plugin#1055

Merged

6 tasks

basil mentioned this pull request Dec 17, 2021

Update dependencies, exclude JSR 305 jenkinsci/remoting#495

Merged

This was referenced Dec 31, 2021

Use findbugs annotations instead of jsr-305 annotations jenkinsci/copyartifact-plugin#144

Merged

Use findbugs annotations (as JSR 305 is dormant) jenkinsci/jira-plugin#397

Merged

EOL JSR 305 jenkinsci/job-dsl-plugin#1246

Merged

MarkEWaite mentioned this pull request Jan 3, 2022

[JENKINS-48466] Provide JUnit 5 support for JenkinsRule jenkinsci/jenkins-test-harness#375

Merged

6 tasks

schottsfired mentioned this pull request Jun 12, 2022

Add JCasC Compatibility Test jenkinsci/snyk-security-scanner-plugin#123

Merged

6 tasks

offa mentioned this pull request Jun 24, 2022

Remove JSR 305 jenkinsci/jobcacher-plugin#52

Merged

6 tasks

basil mentioned this pull request Mar 28, 2024

Replace EOL JSR 305 annotations with SpotBugs annotations jenkinsci/active-choices-plugin#236

Merged

basil mentioned this pull request Nov 13, 2024

SONARJNKNS-381 Fix SSF-682 SonarSource/sonar-scanner-jenkins#301

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kill jsr-305 #4604

Kill jsr-305 #4604

jtnord commented Mar 26, 2020 •

edited

Loading

jtnord commented Mar 26, 2020

timja commented Mar 26, 2020

timja commented Mar 26, 2020

jtnord commented Mar 27, 2020 •

edited

Loading

jtnord commented Mar 27, 2020

timja left a comment

timja Mar 27, 2020

jtnord Mar 27, 2020

basil May 11, 2022

jglick commented Mar 30, 2020

jglick left a comment •

edited

Loading

jglick Mar 30, 2020

jtnord Mar 30, 2020

jeffret-b left a comment

jtnord commented Mar 30, 2020

Kill jsr-305 #4604

Kill jsr-305 #4604

Conversation

jtnord commented Mar 26, 2020 • edited Loading

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

jtnord commented Mar 26, 2020

timja commented Mar 26, 2020

timja commented Mar 26, 2020

jtnord commented Mar 27, 2020 • edited Loading

jtnord commented Mar 27, 2020

timja left a comment

Choose a reason for hiding this comment

timja Mar 27, 2020

Choose a reason for hiding this comment

jtnord Mar 27, 2020

Choose a reason for hiding this comment

basil May 11, 2022

Choose a reason for hiding this comment

jglick commented Mar 30, 2020

jglick left a comment • edited Loading

Choose a reason for hiding this comment

jglick Mar 30, 2020

Choose a reason for hiding this comment

jtnord Mar 30, 2020

Choose a reason for hiding this comment

jeffret-b left a comment

Choose a reason for hiding this comment

jtnord commented Mar 30, 2020

jtnord commented Mar 26, 2020 •

edited

Loading

jtnord commented Mar 27, 2020 •

edited

Loading

jglick left a comment •

edited

Loading