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.

Sign up for GitHub

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

Enable Warnings Next Generation #5752

Merged
merged 4 commits into from
Oct 5, 2021
Merged

Enable Warnings Next Generation #5752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9818d6
Fix Javadoc warnings
basil Sep 27, 2021
60a2ceb
Enable Warnings Next Generation plugin
basil Sep 27, 2021
8c6b19b
Stop recording Maven warnings
basil Oct 3, 2021
c92d07a
Merge remote-tracking branch 'origin/master' into warnings-ng
basil Oct 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ for(j = 0; j < jdks.size(); j++) {
"MAVEN_OPTS=-Xmx1536m -Xms512m"], buildType, jdk) {
// Actually run Maven!
// -Dmaven.repo.local=… tells Maven to create a subdir in the temporary directory for the local Maven repository
def mvnCmd = "mvn -Pdebug -Pjapicmp -U -Dset.changelist help:evaluate -Dexpression=changelist -Doutput=$changelistF clean install ${runTests ? '-Dmaven.test.failure.ignore' : '-DskipTests'} -V -B -ntp -Dmaven.repo.local=$m2repo -e"
def mvnCmd = "mvn -Pdebug -Pjapicmp -U -Dset.changelist help:evaluate -Dexpression=changelist -Doutput=$changelistF clean install ${runTests ? '-Dmaven.test.failure.ignore' : '-DskipTests'} -V -B -ntp -Dmaven.repo.local=$m2repo -Dspotbugs.failOnError=false -Dcheckstyle.failOnViolation=false -e"

if(isUnix()) {
sh mvnCmd
Expand All @@ -66,8 +66,37 @@ for(j = 0; j < jdks.size(); j++) {
if (! fileExists('test/target/surefire-reports/TEST-jenkins.Junit4TestsRanTest.xml') ) {
error 'junit 4 tests are no longer being run for the test package'
} // cli has been migrated to junit 5
if (failFast && currentBuild.result == 'UNSTABLE') {
error 'There were test failures; halting early'
}
}
if (buildType == 'Linux' && jdk == jdks[0]) {
def folders = env.JOB_NAME.split('/')
if (folders.length > 1) {
discoverGitReferenceBuild(scm: folders[1])
}

echo "Recording static analysis results for '${buildType}'"
recordIssues enabledForFailure: true,
tools: [java(), javaDoc()],
filters: [excludeFile('.*Assert.java')],
sourceCodeEncoding: 'UTF-8',
skipBlames: true,
trendChartType: 'TOOLS_ONLY'
recordIssues([tool: spotBugs(pattern: '**/target/spotbugsXml.xml'),
sourceCodeEncoding: 'UTF-8',
skipBlames: true,
trendChartType: 'TOOLS_ONLY',
qualityGates: [[threshold: 1, type: 'NEW', unstable: true]]])
Copy link
Contributor

Choose a reason for hiding this comment

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

With that we could also enable spotbugs on medium and make it a lot harder that new issues get introduced.

Copy link
Member

Choose a reason for hiding this comment

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

We could but how do you reproduce locally is it a matter of enabling medium and searching?

Not a huge fan of the approach but probably better to do so that new issues don’t creep in till we change the level

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a huge fan of the approach

I think you're seeing why I didn't go that far in this PR. :-) Yes, that is a more controversial change. I'm not a fan of using the CI system as a ratchet either: I'd rather use the build system for that. But I don't feel strongly either way. In any case that shouldn't matter for this PR, which preserves the status quo.

recordIssues([tool: checkStyle(pattern: '**/target/checkstyle-result.xml'),
sourceCodeEncoding: 'UTF-8',
skipBlames: true,
trendChartType: 'TOOLS_ONLY',
qualityGates: [[threshold: 1, type: 'TOTAL', unstable: true]]])
if (failFast && currentBuild.result == 'UNSTABLE') {
error 'Static analysis quality gates not passed; halting early'
}

def changelist = readFile(changelistF)
dir(m2repo) {
archiveArtifacts artifacts: "**/*$changelist/*$changelist*",
Expand Down
3 changes: 0 additions & 3 deletions core/src/main/java/hudson/FilePath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2303,9 +2303,6 @@ public String invoke(File f, VirtualChannel channel) throws IOException, Interru
* I/O operations also happens asynchronously from the {@link Channel#call(Callable)} operations, so if
* you write to a remote file and then execute {@link Channel#call(Callable)} and try to access the newly copied
* file, it might not be fully written yet.
*
* <p>
*
*/
public OutputStream write() throws IOException, InterruptedException {
if(channel==null) {
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/hudson/model/UpdateCenter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2116,7 +2116,7 @@ public InstallationJob(Plugin plugin, UpdateSite site, Authentication auth) {
}

/**
* @deprecated use {@link InstallationJob(Plugin, UpdateSite, Authentication, boolean)}
* @deprecated use {@link #InstallationJob(UpdateSite.Plugin, UpdateSite, Authentication, boolean)}
*/
@Deprecated
public InstallationJob(Plugin plugin, UpdateSite site, org.acegisecurity.Authentication auth, boolean dynamicLoad) {
Expand Down Expand Up @@ -2342,7 +2342,7 @@ public final class PluginDowngradeJob extends DownloadJob {
private final PluginManager pm = Jenkins.get().getPluginManager();

/**
* @deprecated use {@link PluginDowngradeJob(Plugin, UpdateSite, Authentication)}
* @deprecated use {@link #PluginDowngradeJob(UpdateSite.Plugin, UpdateSite, Authentication)}
*/
@Deprecated
public PluginDowngradeJob(Plugin plugin, UpdateSite site, org.acegisecurity.Authentication auth) {
Expand Down Expand Up @@ -2439,7 +2439,7 @@ public String toString() {
public final class HudsonUpgradeJob extends DownloadJob {

/**
* @deprecated use {@link HudsonUpgradeJob(UpdateSite site, Authentication auth)}
* @deprecated use {@link #HudsonUpgradeJob(UpdateSite, Authentication)}
*/
@Deprecated
public HudsonUpgradeJob(UpdateSite site, org.acegisecurity.Authentication auth) {
Expand Down Expand Up @@ -2486,7 +2486,7 @@ protected void replace(File dst, File src) throws IOException {
public final class HudsonDowngradeJob extends DownloadJob {

/**
* @deprecated use {@link HudsonDowngradeJob(UpdateSite site, Authentication auth)}
* @deprecated use {@link #HudsonDowngradeJob(UpdateSite, Authentication)}
*/
@Deprecated
public HudsonDowngradeJob(UpdateSite site, org.acegisecurity.Authentication auth) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/User.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
* Unfortunately this infringed some legitimate use cases of creating Jenkins-local users for
* automation purposes. This escape hatch switch can be enabled to resurrect that behaviour.
* <p>
* @see <a href="https://issues.jenkins.io/browse/JENKINS-22346">JENKINS-22346</a>.
* See <a href="https://issues.jenkins.io/browse/JENKINS-22346">JENKINS-22346</a>.
*/
@SuppressFBWarnings("MS_SHOULD_BE_FINAL")
public static boolean ALLOW_NON_EXISTENT_USER_TO_LOGIN = SystemProperties.getBoolean(User.class.getName() + ".allowNonExistentUserToLogin");
Expand Down
9 changes: 4 additions & 5 deletions core/src/main/java/hudson/slaves/CloudProvisioningListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import hudson.model.Label;
import hudson.model.Node;
import hudson.model.queue.CauseOfBlockage;
import hudson.slaves.Cloud.CloudState;
import java.util.Collection;
import java.util.concurrent.Future;
import jenkins.model.Jenkins;
Expand Down Expand Up @@ -36,17 +35,17 @@ public abstract class CloudProvisioningListener implements ExtensionPoint {
* @return {@code null} if provisioning can proceed, or a
* {@link CauseOfBlockage} reason why it cannot be provisioned.
*
* @deprecated Use {@link #canProvision(Cloud, CloudState, int)} instead.
* @deprecated Use {@link #canProvision(Cloud, Cloud.CloudState, int)} )} instead.
*/
@Deprecated
public CauseOfBlockage canProvision(Cloud cloud, Label label, int numExecutors) {
if (Util.isOverridden(CloudProvisioningListener.class,
getClass(),
"canProvision",
Cloud.class,
CloudState.class,
Cloud.CloudState.class,
int.class)) {
return canProvision(cloud, new CloudState(label, 0), numExecutors);
return canProvision(cloud, new Cloud.CloudState(label, 0), numExecutors);
} else {
return null;
}
Expand All @@ -64,7 +63,7 @@ public CauseOfBlockage canProvision(Cloud cloud, Label label, int numExecutors)
* @return {@code null} if provisioning can proceed, or a
* {@link CauseOfBlockage} reason why it cannot be provisioned.
*/
public CauseOfBlockage canProvision(Cloud cloud, CloudState state, int numExecutors) {
public CauseOfBlockage canProvision(Cloud cloud, Cloud.CloudState state, int numExecutors) {
return canProvision(cloud, state.getLabel(), numExecutors);
}

Expand Down
7 changes: 4 additions & 3 deletions core/src/main/java/hudson/slaves/DumbSlave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Descriptor.FormException;
import hudson.model.Node;
import hudson.model.Slave;
import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -42,10 +43,10 @@
public final class DumbSlave extends Slave {
/**
* @deprecated as of 1.286.
* Use {@link #DumbSlave(String, String, String, String, Mode, String, ComputerLauncher, RetentionStrategy, List)}
* Use {@link #DumbSlave(String, String, String, String, Node.Mode, String, ComputerLauncher, RetentionStrategy, List)}
*/
@Deprecated
public DumbSlave(String name, String nodeDescription, String remoteFS, String numExecutors, Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy) throws FormException, IOException {
public DumbSlave(String name, String nodeDescription, String remoteFS, String numExecutors, Node.Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy) throws FormException, IOException {
this(name, nodeDescription, remoteFS, numExecutors, mode, labelString, launcher, retentionStrategy, new ArrayList());
}

Expand All @@ -54,7 +55,7 @@ public DumbSlave(String name, String nodeDescription, String remoteFS, String nu
* Use {@link #DumbSlave(String, String, ComputerLauncher)} and configure the rest through setters.
*/
@Deprecated
public DumbSlave(String name, String nodeDescription, String remoteFS, String numExecutors, Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy, List<? extends NodeProperty<?>> nodeProperties) throws IOException, FormException {
public DumbSlave(String name, String nodeDescription, String remoteFS, String numExecutors, Node.Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy, List<? extends NodeProperty<?>> nodeProperties) throws IOException, FormException {
super(name, nodeDescription, remoteFS, numExecutors, mode, labelString, launcher, retentionStrategy, nodeProperties);
}

Expand Down
9 changes: 4 additions & 5 deletions core/src/main/java/hudson/slaves/NodeProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@
import hudson.model.Descriptor.FormException;
import hudson.model.Environment;
import hudson.model.Node;
import hudson.model.Queue.BuildableItem;
import hudson.model.Queue.Task;
import hudson.model.Queue;
import hudson.model.ReconfigurableDescribable;
import hudson.model.TaskListener;
import hudson.model.queue.CauseOfBlockage;
Expand Down Expand Up @@ -95,10 +94,10 @@ public NodePropertyDescriptor getDescriptor() {
*
* @since 1.360
* @deprecated as of 1.413
* Use {@link #canTake(BuildableItem)}
* Use {@link #canTake(Queue.BuildableItem)}
*/
@Deprecated
public CauseOfBlockage canTake(Task task) {
public CauseOfBlockage canTake(Queue.Task task) {
return null;
}

Expand All @@ -110,7 +109,7 @@ public CauseOfBlockage canTake(Task task) {
*
* @since 1.413
*/
public CauseOfBlockage canTake(BuildableItem item) {
public CauseOfBlockage canTake(Queue.BuildableItem item) {
return canTake(item.task); // backward compatible behaviour
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/jenkins/util/io/CompositeIOException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public CompositeIOException(String message, @NonNull List<IOException> exception
}

/**
* @see CompositeIOException(String, List)
* @see #CompositeIOException(String, List)
*/
public CompositeIOException(String message, IOException... exceptions) {
this(message, Arrays.asList(exceptions));
Expand Down
1 change: 0 additions & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,6 @@ THE SOFTWARE.
<!-- Version specified in parent POM -->
<configuration>
<consoleOutput>true</consoleOutput>
<failsOnError>true</failsOnError>
Copy link
Member

Choose a reason for hiding this comment

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

This is for the Checkstyle plugin, right? Would be nice to retain this behavior for non-CI builds, e.g. via a default profile

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for the Checkstyle plugin, right?

Yes.

Would be nice to retain this behavior for non-CI builds

The existing behavior is retained for non-CI builds via <failOnViolation>, which defaults to true. <failsOnError> is unnecessary to retain the existing behavior.

<includeTestSourceDirectory>true</includeTestSourceDirectory>
<!-- define here because checkstyle and multi module is a PITA -->
<checkstyleRules>
Expand Down