From 23f3c9de3db9a6147ae268a4018966db521cbc7e Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 10:59:48 +0100 Subject: [PATCH 1/6] I disagree with Sonar - moving the assignment make the code worse --- src/main/org/apache/tools/ant/taskdefs/Expand.java | 2 +- .../org/apache/tools/ant/taskdefs/optional/NetRexxC.java | 2 +- src/main/org/apache/tools/bzip2/BlockSort.java | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/org/apache/tools/ant/taskdefs/Expand.java b/src/main/org/apache/tools/ant/taskdefs/Expand.java index cb0c958d5..0ec233308 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Expand.java +++ b/src/main/org/apache/tools/ant/taskdefs/Expand.java @@ -191,7 +191,7 @@ public class Expand extends Task { log("extracting " + ze.getName(), Project.MSG_DEBUG); try { extractFile(fileUtils, srcF, dir, - is = zf.getInputStream(ze), + is = zf.getInputStream(ze), //NOSONAR ze.getName(), new Date(ze.getTime()), ze.isDirectory(), mapper); } finally { diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/NetRexxC.java b/src/main/org/apache/tools/ant/taskdefs/optional/NetRexxC.java index 5ba2a760e..21440f1e6 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/NetRexxC.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/NetRexxC.java @@ -868,7 +868,7 @@ public class NetRexxC extends MatchingTask { PrintWriter w = null; int rc = COM.ibm.netrexx.process.NetRexxC.main(new Rexx(compileArgs), - w = new PrintWriter(out)); + w = new PrintWriter(out)); //NOSONAR String sdir = srcDir.getAbsolutePath(); String ddir = destDir.getAbsolutePath(); boolean doReplace = !(sdir.equals(ddir)); diff --git a/src/main/org/apache/tools/bzip2/BlockSort.java b/src/main/org/apache/tools/bzip2/BlockSort.java index eb9066ee9..78ea15e31 100644 --- a/src/main/org/apache/tools/bzip2/BlockSort.java +++ b/src/main/org/apache/tools/bzip2/BlockSort.java @@ -642,7 +642,7 @@ class BlockSort { HAMMER: while (true) { if (onceRunned) { fmap[j] = a; - if ((j -= h) <= mj) { + if ((j -= h) <= mj) { //NOSONAR break HAMMER; } } else { @@ -660,7 +660,7 @@ class BlockSort { if (block[i1 + 3] == block[i2 + 3]) { if (block[i1 + 4] == block[i2 + 4]) { if (block[i1 + 5] == block[i2 + 5]) { - if (block[(i1 += 6)] == block[(i2 += 6)]) { + if (block[(i1 += 6)] == block[(i2 += 6)]) { //NOSONAR int x = lastShadow; X: while (x > 0) { x -= 4; @@ -673,10 +673,10 @@ class BlockSort { if (quadrant[i1 + 2] == quadrant[i2 + 2]) { if (block[i1 + 4] == block[i2 + 4]) { if (quadrant[i1 + 3] == quadrant[i2 + 3]) { - if ((i1 += 4) >= lastPlus1) { + if ((i1 += 4) >= lastPlus1) { //NOSONAR i1 -= lastPlus1; } - if ((i2 += 4) >= lastPlus1) { + if ((i2 += 4) >= lastPlus1) { //NOSONAR i2 -= lastPlus1; } workDoneShadow++; From 07ce505d5338388a069666e10aa4644b16798056 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 11:00:48 +0100 Subject: [PATCH 2/6] extract assignment statements --- src/main/org/apache/tools/ant/filters/UniqFilter.java | 7 +++++-- src/main/org/apache/tools/ant/taskdefs/Sync.java | 3 ++- .../ant/taskdefs/optional/junit/AggregateTransformer.java | 6 ++++-- src/main/org/apache/tools/bzip2/BlockSort.java | 6 ++++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/main/org/apache/tools/ant/filters/UniqFilter.java b/src/main/org/apache/tools/ant/filters/UniqFilter.java index e72d5f581..2983b6c90 100644 --- a/src/main/org/apache/tools/ant/filters/UniqFilter.java +++ b/src/main/org/apache/tools/ant/filters/UniqFilter.java @@ -31,7 +31,10 @@ public class UniqFilter extends TokenFilter.ChainableReaderFilter { private String lastLine = null; public String filter(String string) { - return lastLine == null || !lastLine.equals(string) - ? (lastLine = string) : null; + if (lastLine == null || !lastLine.equals(string)) { + lastLine = string; + return lastLine; + } + return null; } } diff --git a/src/main/org/apache/tools/ant/taskdefs/Sync.java b/src/main/org/apache/tools/ant/taskdefs/Sync.java index c1c755291..bcdfd525c 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Sync.java +++ b/src/main/org/apache/tools/ant/taskdefs/Sync.java @@ -403,7 +403,8 @@ public class Sync extends Task { if (resources == null) { Restrict r = new Restrict(); r.add(new Exists()); - r.add(resources = new Resources()); + resources = new Resources(); + r.add(resources); myCopy.add(r); } resources.add(rc); diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/AggregateTransformer.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/AggregateTransformer.java index bae3b3ca2..fd7a321eb 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/AggregateTransformer.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/junit/AggregateTransformer.java @@ -239,8 +239,10 @@ public class AggregateTransformer { * @since Ant 1.9.5 */ public XSLTProcess.Factory createFactory() { - return xsltFactory != null ? xsltFactory - : (xsltFactory = xsltTask.createFactory()); + if (xsltFactory == null) { + xsltFactory = xsltTask.createFactory(); + } + return xsltFactory; } /** diff --git a/src/main/org/apache/tools/bzip2/BlockSort.java b/src/main/org/apache/tools/bzip2/BlockSort.java index 78ea15e31..b382470f8 100644 --- a/src/main/org/apache/tools/bzip2/BlockSort.java +++ b/src/main/org/apache/tools/bzip2/BlockSort.java @@ -445,8 +445,10 @@ class BlockSort { private int[] eclass; private int[] getEclass() { - return eclass == null - ? (eclass = new int[quadrant.length / 2]) : eclass; + if (eclass == null) { + eclass = new int[quadrant.length / 2]; + } + return eclass; } /* From 144fc493a9f9ee351424721462de538677821343 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 11:11:40 +0100 Subject: [PATCH 3/6] objects created for side effects only --- src/main/org/apache/tools/ant/helper/ProjectHelperImpl.java | 3 ++- src/main/org/apache/tools/ant/taskdefs/Javadoc.java | 3 ++- src/main/org/apache/tools/ant/taskdefs/MacroDef.java | 4 +++- .../org/apache/tools/ant/taskdefs/condition/AntVersion.java | 6 ++++-- .../apache/tools/ant/util/depend/bcel/AncestorAnalyzer.java | 2 +- .../org/apache/tools/ant/util/depend/bcel/FullAnalyzer.java | 2 +- 6 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/org/apache/tools/ant/helper/ProjectHelperImpl.java b/src/main/org/apache/tools/ant/helper/ProjectHelperImpl.java index f828d2926..97c42a028 100644 --- a/src/main/org/apache/tools/ant/helper/ProjectHelperImpl.java +++ b/src/main/org/apache/tools/ant/helper/ProjectHelperImpl.java @@ -600,7 +600,8 @@ public class ProjectHelperImpl extends ProjectHelper { private static void handleElement(ProjectHelperImpl helperImpl, DocumentHandler parent, Target target, String elementName, AttributeList attrs) throws SAXParseException { if (elementName.equals("description")) { - new DescriptionHandler(helperImpl, parent); + // created for side effect + new DescriptionHandler(helperImpl, parent); //NOSONAR } else if (helperImpl.project.getDataTypeDefinitions().get(elementName) != null) { new DataTypeHandler(helperImpl, parent, target).init(elementName, attrs); } else { diff --git a/src/main/org/apache/tools/ant/taskdefs/Javadoc.java b/src/main/org/apache/tools/ant/taskdefs/Javadoc.java index b7fb41772..9fd3c8a3f 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Javadoc.java +++ b/src/main/org/apache/tools/ant/taskdefs/Javadoc.java @@ -2026,7 +2026,8 @@ public class Javadoc extends Task { // is the href a valid URL try { final URL base = new URL("file://."); - new URL(base, la.getHref()); + // created for the side effect of throwing a MalformedURLException + new URL(base, la.getHref()); //NOSONAR link = la.getHref(); } catch (final MalformedURLException mue) { // ok - just skip diff --git a/src/main/org/apache/tools/ant/taskdefs/MacroDef.java b/src/main/org/apache/tools/ant/taskdefs/MacroDef.java index 63f68c5a8..95757b6df 100644 --- a/src/main/org/apache/tools/ant/taskdefs/MacroDef.java +++ b/src/main/org/apache/tools/ant/taskdefs/MacroDef.java @@ -183,7 +183,9 @@ public class MacroDef extends AntlibDefinition { ret.setTaskName("sequential"); ret.setNamespace(""); ret.setQName("sequential"); - new RuntimeConfigurable(ret, "sequential"); + // stores RuntimeConfigurable as "RuntimeConfigurableWrapper" + // in ret as side effect + new RuntimeConfigurable(ret, "sequential"); //NOSONAR final int size = nestedSequential.getNested().size(); for (int i = 0; i < size; ++i) { UnknownElement e = diff --git a/src/main/org/apache/tools/ant/taskdefs/condition/AntVersion.java b/src/main/org/apache/tools/ant/taskdefs/condition/AntVersion.java index aadf5a7e5..ec21d4b48 100644 --- a/src/main/org/apache/tools/ant/taskdefs/condition/AntVersion.java +++ b/src/main/org/apache/tools/ant/taskdefs/condition/AntVersion.java @@ -78,7 +78,8 @@ public class AntVersion extends Task implements Condition { } if (atLeast != null) { try { - new DeweyDecimal(atLeast); + // only created for side effect + new DeweyDecimal(atLeast); //NOSONAR } catch (NumberFormatException e) { throw new BuildException( "The 'atleast' attribute is not a Dewey Decimal eg 1.1.0 : " @@ -86,7 +87,8 @@ public class AntVersion extends Task implements Condition { } } else { try { - new DeweyDecimal(exactly); + // only created for side effect + new DeweyDecimal(exactly); //NOSONAR } catch (NumberFormatException e) { throw new BuildException( "The 'exactly' attribute is not a Dewey Decimal eg 1.1.0 : " diff --git a/src/main/org/apache/tools/ant/util/depend/bcel/AncestorAnalyzer.java b/src/main/org/apache/tools/ant/util/depend/bcel/AncestorAnalyzer.java index 2bd2a6cf6..613bc77a2 100644 --- a/src/main/org/apache/tools/ant/util/depend/bcel/AncestorAnalyzer.java +++ b/src/main/org/apache/tools/ant/util/depend/bcel/AncestorAnalyzer.java @@ -42,7 +42,7 @@ public class AncestorAnalyzer extends AbstractAnalyzer { public AncestorAnalyzer() { // force BCEL classes to load now try { - new ClassParser("force"); + new ClassParser("force"); //NOSONAR } catch (Exception e) { // all released versions of BCEL may throw an IOException // here, but BCEL's trunk does no longer declare to do so diff --git a/src/main/org/apache/tools/ant/util/depend/bcel/FullAnalyzer.java b/src/main/org/apache/tools/ant/util/depend/bcel/FullAnalyzer.java index f270fd48e..3bd6c752c 100644 --- a/src/main/org/apache/tools/ant/util/depend/bcel/FullAnalyzer.java +++ b/src/main/org/apache/tools/ant/util/depend/bcel/FullAnalyzer.java @@ -41,7 +41,7 @@ public class FullAnalyzer extends AbstractAnalyzer { public FullAnalyzer() { // force BCEL classes to load now try { - new ClassParser("force"); + new ClassParser("force"); //NOSONAR } catch (Exception e) { // all released versions of BCEL may throw an IOException // here, but BCEL's trunk does no longer declare to do so From eaf3af0aea8b14c4a5d8d63bc4cb22f88dbb19a4 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 11:22:00 +0100 Subject: [PATCH 4/6] Sonar falsely claims expressions to be constant --- src/main/org/apache/tools/ant/ArgumentProcessorRegistry.java | 2 +- src/main/org/apache/tools/ant/taskdefs/StreamPumper.java | 2 +- .../org/apache/tools/ant/taskdefs/optional/net/RExecTask.java | 2 +- .../org/apache/tools/ant/taskdefs/optional/net/TelnetTask.java | 2 +- src/main/org/apache/tools/ant/taskdefs/optional/ssh/Scp.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/org/apache/tools/ant/ArgumentProcessorRegistry.java b/src/main/org/apache/tools/ant/ArgumentProcessorRegistry.java index ae95c3b79..0c7c35bd0 100644 --- a/src/main/org/apache/tools/ant/ArgumentProcessorRegistry.java +++ b/src/main/org/apache/tools/ant/ArgumentProcessorRegistry.java @@ -84,7 +84,7 @@ public class ArgumentProcessorRegistry { } InputStream systemResource = ClassLoader.getSystemResourceAsStream(SERVICE_ID); - if (systemResource != null) { + if (systemResource != null) { //NOSONAR ArgumentProcessor processor = getProcessorByService(systemResource); registerArgumentProcessor(processor); } diff --git a/src/main/org/apache/tools/ant/taskdefs/StreamPumper.java b/src/main/org/apache/tools/ant/taskdefs/StreamPumper.java index df95eedb6..59d886b79 100644 --- a/src/main/org/apache/tools/ant/taskdefs/StreamPumper.java +++ b/src/main/org/apache/tools/ant/taskdefs/StreamPumper.java @@ -137,7 +137,7 @@ public class StreamPumper implements Runnable { if (autoflush) { os.flush(); } - if (finish) { + if (finish) { //NOSONAR break; } } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/net/RExecTask.java b/src/main/org/apache/tools/ant/taskdefs/optional/net/RExecTask.java index 61d6e3ddd..d88874f25 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/net/RExecTask.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/net/RExecTask.java @@ -362,7 +362,7 @@ public class RExecTask extends Task { } catch (IOException e) { throw new BuildException("Can't connect to " + server); } - if (userid != null && password != null && command != null + if (userid != null && password != null && command != null //NOSONAR && rexecTasks.size() == 0) { // simple one-shot execution rexec.rexec(userid, password, command); diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/net/TelnetTask.java b/src/main/org/apache/tools/ant/taskdefs/optional/net/TelnetTask.java index 2bd220540..a89e7a9c8 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/net/TelnetTask.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/net/TelnetTask.java @@ -107,7 +107,7 @@ public class TelnetTask extends Task { throw new BuildException("Can't connect to " + server); } /** Login if userid and password were specified */ - if (userid != null && password != null) { + if (userid != null && password != null) { //NOSONAR login(telnet); } /** Process each sub command */ diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/ssh/Scp.java b/src/main/org/apache/tools/ant/taskdefs/optional/ssh/Scp.java index d2a09bc68..087a4022b 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/ssh/Scp.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/ssh/Scp.java @@ -267,7 +267,7 @@ public class Scp extends SSHBase { } else { upload(fromUri, toUri); } - } else if (isFromRemote && isToRemote) { + } else if (isFromRemote && isToRemote) { //NOSONAR throw new BuildException( "Copying from a remote server to a remote server is not supported."); } else { From 64d1056c41516ea0a35e8b7c9d9efaab4153027a Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 11:22:23 +0100 Subject: [PATCH 5/6] uncover hidden NullPointerException --- src/main/org/apache/tools/ant/taskdefs/Jar.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/org/apache/tools/ant/taskdefs/Jar.java b/src/main/org/apache/tools/ant/taskdefs/Jar.java index a0337e1f1..330b5dd7d 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Jar.java +++ b/src/main/org/apache/tools/ant/taskdefs/Jar.java @@ -413,7 +413,7 @@ public class Jar extends Zip { */ public void setFilesetmanifest(FilesetManifestConfig config) { filesetManifestConfig = config; - mergeManifestsMain = "merge".equals(config.getValue()); + mergeManifestsMain = config != null && "merge".equals(config.getValue()); if (filesetManifestConfig != null && !filesetManifestConfig.getValue().equals("skip")) { From db620447093f2cf3b83cdc382cdd2be74cd39067 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Wed, 21 Dec 2016 11:22:41 +0100 Subject: [PATCH 6/6] unnecessary condition --- src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java b/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java index f9c53eaab..2e2f005ea 100644 --- a/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java +++ b/src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java @@ -831,7 +831,7 @@ public class XSLTProcess extends MatchingTask implements XSLTLogger { if (outFileName == null || outFileName.length == 0) { log("Skipping " + inFile + " it cannot get mapped to output.", Project.MSG_VERBOSE); return; - } else if (outFileName == null || outFileName.length > 1) { + } else if (outFileName.length > 1) { log("Skipping " + inFile + " its mapping is ambiguos.", Project.MSG_VERBOSE); return; }