From 1d614c3a7ecf8a3ede4df8a50da46e71792d0025 Mon Sep 17 00:00:00 2001 From: Ivan Sopov Date: Thu, 5 Dec 2013 19:12:51 +0200 Subject: [PATCH] poartial fix for #46 - Sonarqube found problems in Checkstyle --- .../tools/checkstyle/CheckStyleTask.java | 4 ++-- .../puppycrawl/tools/checkstyle/Checker.java | 8 +++++--- .../checkstyle/api/MessageDispatcher.java | 4 ++-- .../checks/blocks/EmptyBlockCheck.java | 14 +++++++------- .../checks/coding/CovariantEqualsCheck.java | 16 ++++++++-------- .../checks/coding/EqualsAvoidNullCheck.java | 19 +++++++++++-------- .../checks/coding/FallThroughCheck.java | 16 ++++++++-------- .../coding/OneStatementPerLineCheck.java | 9 +++++---- .../checks/coding/RedundantThrowsCheck.java | 8 +++----- .../duplicates/StrictDuplicateCodeCheck.java | 7 ++----- .../checks/imports/ImportControlLoader.java | 4 ++-- .../checks/imports/ImportOrderCheck.java | 6 ++---- .../checks/javadoc/JavadocMethodCheck.java | 10 ++++------ .../checkstyle/checks/javadoc/TagParser.java | 2 +- .../checks/whitespace/ParenPadCheck.java | 7 +++---- .../checkstyle/filters/SuppressElement.java | 14 ++++---------- .../SuppressWithNearbyCommentFilter.java | 7 +++---- 17 files changed, 72 insertions(+), 83 deletions(-) diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java index 7417fd10da5..e655a0eeb18 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/CheckStyleTask.java @@ -284,7 +284,7 @@ private void realExecute() log("compiled on " + compileTimestamp, Project.MSG_VERBOSE); // Check for no arguments - if ((mFileName == null) && (mFileSets.size() == 0)) { + if ((mFileName == null) && mFileSets.isEmpty()) { throw new BuildException( "Must specify at least one of 'file' or nested 'fileset'.", getLocation()); @@ -449,7 +449,7 @@ protected AuditListener[] getListeners() throws ClassNotFoundException, final AuditListener[] listeners = new AuditListener[formatterCount]; // formatters - if (mFormatters.size() == 0) { + if (mFormatters.isEmpty()) { final OutputStream debug = new LogOutputStream(this, Project.MSG_DEBUG); final OutputStream err = new LogOutputStream(this, Project.MSG_ERR); diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java index 74eae23de5e..f8d96afafa8 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java @@ -36,6 +36,7 @@ import com.puppycrawl.tools.checkstyle.api.SeverityLevel; import com.puppycrawl.tools.checkstyle.api.SeverityLevelCounter; import com.puppycrawl.tools.checkstyle.api.Utils; + import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -44,8 +45,8 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.SortedSet; import java.util.StringTokenizer; -import java.util.TreeSet; /** * This class provides the functionality to check a set of files. @@ -254,7 +255,7 @@ public int process(List aFiles) for (final File f : aFiles) { final String fileName = f.getAbsolutePath(); fireFileStarted(fileName); - final TreeSet fileMessages = Sets.newTreeSet(); + final SortedSet fileMessages = Sets.newTreeSet(); try { final FileText theText = new FileText(f.getAbsoluteFile(), mCharset); @@ -501,7 +502,8 @@ public void fireFileFinished(String aFileName) * @param aFileName the audited file * @param aErrors the audit errors from the file */ - public void fireErrors(String aFileName, TreeSet aErrors) + public void fireErrors(String aFileName, + SortedSet aErrors) { final String stripped = getStrippedFileName(aFileName); for (final LocalizedMessage element : aErrors) { diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/api/MessageDispatcher.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/api/MessageDispatcher.java index a4d50c1b46b..59927ba838f 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/api/MessageDispatcher.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/api/MessageDispatcher.java @@ -18,7 +18,7 @@ //////////////////////////////////////////////////////////////////////////////// package com.puppycrawl.tools.checkstyle.api; -import java.util.TreeSet; +import java.util.SortedSet; /** * Used by FileSetChecks to distribute AuditEvents to AuditListeners. @@ -43,5 +43,5 @@ public interface MessageDispatcher * @param aFileName the audited file * @param aErrors the audit errors from the file */ - void fireErrors(String aFileName, TreeSet aErrors); + void fireErrors(String aFileName, SortedSet aErrors); } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheck.java index 43638796776..c420d384436 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheck.java @@ -101,13 +101,13 @@ public void visitToken(DetailAST aAST) aAST.getText()); } } - else if (getAbstractOption() == BlockOption.TEXT) { - if (!hasText(slistAST)) { - log(slistAST.getLineNo(), - slistAST.getColumnNo(), - "block.empty", - aAST.getText()); - } + else if (getAbstractOption() == BlockOption.TEXT + && !hasText(slistAST)) + { + log(slistAST.getLineNo(), + slistAST.getColumnNo(), + "block.empty", + aAST.getText()); } } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/CovariantEqualsCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/CovariantEqualsCheck.java index a44729fde19..ca1e87175e6 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/CovariantEqualsCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/CovariantEqualsCheck.java @@ -69,14 +69,14 @@ public void visitToken(DetailAST aAST) if (objBlock != null) { DetailAST child = objBlock.getFirstChild(); while (child != null) { - if (child.getType() == TokenTypes.METHOD_DEF) { - if (CheckUtils.isEqualsMethod(child)) { - if (hasObjectParameter(child)) { - hasEqualsObject = true; - } - else { - mEqualsMethods.add(child); - } + if (child.getType() == TokenTypes.METHOD_DEF + && CheckUtils.isEqualsMethod(child)) + { + if (hasObjectParameter(child)) { + hasEqualsObject = true; + } + else { + mEqualsMethods.add(child); } } child = child.getNextSibling(); diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java index b53552d0f39..e670bd38b6b 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java @@ -123,15 +123,18 @@ public void visitToken(final DetailAST aMethodCall) final DetailAST expr = dot.getNextSibling().getFirstChild(); if ("equals".equals(method.getText()) - || (!mIgnoreEqualsIgnoreCase && "equalsIgnoreCase" - .equals(method.getText()))) + && containsOneArg(expr) && containsAllSafeTokens(expr)) { - if (containsOneArg(expr) && containsAllSafeTokens(expr)) { - log(aMethodCall.getLineNo(), aMethodCall.getColumnNo(), - "equals".equals(method.getText()) - ? "equals.avoid.null" - : "equalsIgnoreCase.avoid.null"); - } + log(aMethodCall.getLineNo(), aMethodCall.getColumnNo(), + "equals.avoid.null"); + } + + if (!mIgnoreEqualsIgnoreCase + && "equalsIgnoreCase".equals(method.getText()) + && containsOneArg(expr) && containsAllSafeTokens(expr)) + { + log(aMethodCall.getLineNo(), aMethodCall.getColumnNo(), + "equalsIgnoreCase.avoid.null"); } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java index 75e47550851..a85075fa6ed 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java @@ -133,14 +133,14 @@ public void visitToken(DetailAST aAST) final DetailAST slist = aAST.findFirstToken(TokenTypes.SLIST); - if (!isTerminated(slist, true, true)) { - if (!hasFallTruComment(aAST, nextGroup)) { - if (!isLastGroup) { - log(nextGroup, "fall.through"); - } - else { - log(aAST, "fall.through.last"); - } + if (!isTerminated(slist, true, true) + && !hasFallTruComment(aAST, nextGroup)) + { + if (!isLastGroup) { + log(nextGroup, "fall.through"); + } + else { + log(aAST, "fall.through.last"); } } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/OneStatementPerLineCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/OneStatementPerLineCheck.java index 79e8917f91f..1e81e7972ae 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/OneStatementPerLineCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/OneStatementPerLineCheck.java @@ -99,10 +99,11 @@ public void leaveToken(DetailAST aAst) private void visitExpr(DetailAST aAst) { mExprDepth++; - if (mExprDepth == 1) { - if (!mInForHeader && (mLastStatementEnd == aAst.getLineNo())) { - log(aAst, "multiple.statements.line"); - } + if (mExprDepth == 1 + && !mInForHeader + && (mLastStatementEnd == aAst.getLineNo())) + { + log(aAst, "multiple.statements.line"); } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java index 431c53b1722..f96eb992d26 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RedundantThrowsCheck.java @@ -134,11 +134,9 @@ private void checkException(FullIdent aExc, List aKnownExcs) final ClassInfo newClassInfo = createClassInfo(new Token(aExc), getCurrentClassName()); - if (!mAllowUnchecked) { - if (isUnchecked(newClassInfo.getClazz())) { - log(aExc.getLineNo(), aExc.getColumnNo(), - "redundant.throws.unchecked", aExc.getText()); - } + if (!mAllowUnchecked && isUnchecked(newClassInfo.getClazz())) { + log(aExc.getLineNo(), aExc.getColumnNo(), + "redundant.throws.unchecked", aExc.getText()); } boolean shouldAdd = true; diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/duplicates/StrictDuplicateCodeCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/duplicates/StrictDuplicateCodeCheck.java index 19e61a0ed5c..72e6c6147fa 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/duplicates/StrictDuplicateCodeCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/duplicates/StrictDuplicateCodeCheck.java @@ -430,11 +430,8 @@ private void findDuplicateFromLine( } final Collection ignoreEntries = aIgnore.get(aILine); - // avoid Integer constructor whenever we can - if (ignoreEntries != null) { - if (ignoreEntries.contains(jLine)) { - continue; - } + if (ignoreEntries != null && ignoreEntries.contains(jLine)) { + continue; } final int duplicateLines = diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java index c5d6f48c0a5..d053178a0f2 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlLoader.java @@ -89,12 +89,12 @@ public void startElement(final String aNamespaceURI, mStack.push(new PkgControl(pkg)); } else if ("subpackage".equals(aQName)) { - assert mStack.size() > 0; + assert !mStack.isEmpty(); final String name = safeGet(aAtts, "name"); mStack.push(new PkgControl(mStack.peek(), name)); } else if ("allow".equals(aQName) || "disallow".equals(aQName)) { - assert mStack.size() > 0; + assert !mStack.isEmpty(); // Need to handle either "pkg" or "class" attribute. // May have "exact-match" for "pkg" // May have "local-only" diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheck.java index 86eb0a68913..2c36a78be15 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheck.java @@ -294,8 +294,7 @@ private void doVisitToken(FullIdent aIdent, boolean aIsStatic, } } else if (groupIdx == mLastGroup) { - doVisitTokenInSameGroup(aIdent, aIsStatic, aPrevious, name, - line); + doVisitTokenInSameGroup(aIsStatic, aPrevious, name, line); } else { log(line, "import.ordering", name); @@ -309,14 +308,13 @@ else if (groupIdx == mLastGroup) { /** * Shares processing... * - * @param aIdent the import to process. * @param aIsStatic whether the token is static or not. * @param aPrevious previous non-static but current is static (above), or * previous static but current is non-static (under). * @param aName the name of the current import. * @param aLine the line of the current import. */ - private void doVisitTokenInSameGroup(FullIdent aIdent, boolean aIsStatic, + private void doVisitTokenInSameGroup(boolean aIsStatic, boolean aPrevious, String aName, int aLine) { if (!mOrdered) { diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java index d23a239b273..d90cbb8df69 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java @@ -260,7 +260,7 @@ protected final void processAST(DetailAST aAST) } } else { - checkComment(aAST, cmt, theScope); + checkComment(aAST, cmt); } } } @@ -313,13 +313,12 @@ private boolean shouldCheck(final DetailAST aAST, final Scope aScope) * * @param aAST the token for the method * @param aComment the Javadoc comment - * @param aScope the scope of the method. */ - private void checkComment(DetailAST aAST, TextBlock aComment, Scope aScope) + private void checkComment(DetailAST aAST, TextBlock aComment) { final List tags = getMethodTags(aComment); - if (hasShortCircuitTag(aAST, tags, aScope)) { + if (hasShortCircuitTag(aAST, tags)) { return; } @@ -354,11 +353,10 @@ private void checkComment(DetailAST aAST, TextBlock aComment, Scope aScope) * * @param aAST the construct being checked * @param aTags the list of Javadoc tags associated with the construct - * @param aScope the scope of the construct * @return true if the construct has a short circuit tag. */ private boolean hasShortCircuitTag(final DetailAST aAST, - final List aTags, final Scope aScope) + final List aTags) { // Check if it contains {@inheritDoc} tag if ((aTags.size() != 1) diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java index acca9215554..7f42a1fa714 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java @@ -74,7 +74,7 @@ public HtmlTag nextTag() */ public boolean hasNextTag() { - return (mTags.size() > 0); + return !mTags.isEmpty(); } /** diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java index 15e0a58a804..44b372c9f67 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/whitespace/ParenPadCheck.java @@ -91,14 +91,13 @@ public void visitToken(DetailAST aAST) processLeft(theAst); } } - else if ((theAst.getParent() == null) + else if (((theAst.getParent() == null) || (theAst.getParent().getType() != TokenTypes.TYPECAST) || (theAst.getParent().findFirstToken(TokenTypes.RPAREN) != theAst)) + && !isFollowsEmptyForIterator(theAst)) { - if (!isFollowsEmptyForIterator(theAst)) { - processRight(theAst); - } + processRight(theAst); } } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java index 82d234258e2..ecd31359531 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java @@ -154,18 +154,12 @@ public boolean accept(AuditEvent aEvent) return false; } - // reject if line matches a line CSV value. - if (mLineFilter != null) { - if (mLineFilter.accept(aEvent.getLine())) { - return false; - } + if (mLineFilter != null && mLineFilter.accept(aEvent.getLine())) { + return false; } - // reject if column matches a column CSV value. - if (mColumnFilter != null) { - if (mColumnFilter.accept(aEvent.getColumn())) { - return false; - } + if (mColumnFilter != null && mColumnFilter.accept(aEvent.getColumn())) { + return false; } return true; } diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java index d615dcfbbce..adf31fcee17 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java @@ -484,9 +484,9 @@ private void tagSuppressions(Collection aComments) for (final TextBlock comment : aComments) { final int startLineNo = comment.getStartLineNo(); final String[] text = comment.getText(); - tagCommentLine(text[0], startLineNo, comment.getStartColNo()); + tagCommentLine(text[0], startLineNo); for (int i = 1; i < text.length; i++) { - tagCommentLine(text[i], startLineNo + i, 0); + tagCommentLine(text[i], startLineNo + i); } } } @@ -496,9 +496,8 @@ private void tagSuppressions(Collection aComments) * checkstyle reporting on or the format for turning reporting off. * @param aText the string to tag. * @param aLine the line number of aText. - * @param aColumn the column number of aText. */ - private void tagCommentLine(String aText, int aLine, int aColumn) + private void tagCommentLine(String aText, int aLine) { final Matcher matcher = mCommentRegexp.matcher(aText); if (matcher.find()) {