Skip to content

Commit

Permalink
poartial fix for #46 - Sonarqube found problems in Checkstyle
Browse files Browse the repository at this point in the history
  • Loading branch information
isopov committed Dec 5, 2013
1 parent 5ce771a commit 1d614c3
Show file tree
Hide file tree
Showing 17 changed files with 72 additions and 83 deletions.
Expand Up @@ -284,7 +284,7 @@ private void realExecute()
log("compiled on " + compileTimestamp, Project.MSG_VERBOSE); log("compiled on " + compileTimestamp, Project.MSG_VERBOSE);


// Check for no arguments // Check for no arguments
if ((mFileName == null) && (mFileSets.size() == 0)) { if ((mFileName == null) && mFileSets.isEmpty()) {
throw new BuildException( throw new BuildException(
"Must specify at least one of 'file' or nested 'fileset'.", "Must specify at least one of 'file' or nested 'fileset'.",
getLocation()); getLocation());
Expand Down Expand Up @@ -449,7 +449,7 @@ protected AuditListener[] getListeners() throws ClassNotFoundException,
final AuditListener[] listeners = new AuditListener[formatterCount]; final AuditListener[] listeners = new AuditListener[formatterCount];


// formatters // formatters
if (mFormatters.size() == 0) { if (mFormatters.isEmpty()) {
final OutputStream debug = new LogOutputStream(this, final OutputStream debug = new LogOutputStream(this,
Project.MSG_DEBUG); Project.MSG_DEBUG);
final OutputStream err = new LogOutputStream(this, Project.MSG_ERR); final OutputStream err = new LogOutputStream(this, Project.MSG_ERR);
Expand Down
8 changes: 5 additions & 3 deletions src/checkstyle/com/puppycrawl/tools/checkstyle/Checker.java
Expand Up @@ -36,6 +36,7 @@
import com.puppycrawl.tools.checkstyle.api.SeverityLevel; import com.puppycrawl.tools.checkstyle.api.SeverityLevel;
import com.puppycrawl.tools.checkstyle.api.SeverityLevelCounter; import com.puppycrawl.tools.checkstyle.api.SeverityLevelCounter;
import com.puppycrawl.tools.checkstyle.api.Utils; import com.puppycrawl.tools.checkstyle.api.Utils;

import java.io.File; import java.io.File;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
Expand All @@ -44,8 +45,8 @@
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Set; import java.util.Set;
import java.util.SortedSet;
import java.util.StringTokenizer; import java.util.StringTokenizer;
import java.util.TreeSet;


/** /**
* This class provides the functionality to check a set of files. * This class provides the functionality to check a set of files.
Expand Down Expand Up @@ -254,7 +255,7 @@ public int process(List<File> aFiles)
for (final File f : aFiles) { for (final File f : aFiles) {
final String fileName = f.getAbsolutePath(); final String fileName = f.getAbsolutePath();
fireFileStarted(fileName); fireFileStarted(fileName);
final TreeSet<LocalizedMessage> fileMessages = Sets.newTreeSet(); final SortedSet<LocalizedMessage> fileMessages = Sets.newTreeSet();
try { try {
final FileText theText = new FileText(f.getAbsoluteFile(), final FileText theText = new FileText(f.getAbsoluteFile(),
mCharset); mCharset);
Expand Down Expand Up @@ -501,7 +502,8 @@ public void fireFileFinished(String aFileName)
* @param aFileName the audited file * @param aFileName the audited file
* @param aErrors the audit errors from the file * @param aErrors the audit errors from the file
*/ */
public void fireErrors(String aFileName, TreeSet<LocalizedMessage> aErrors) public void fireErrors(String aFileName,
SortedSet<LocalizedMessage> aErrors)
{ {
final String stripped = getStrippedFileName(aFileName); final String stripped = getStrippedFileName(aFileName);
for (final LocalizedMessage element : aErrors) { for (final LocalizedMessage element : aErrors) {
Expand Down
Expand Up @@ -18,7 +18,7 @@
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
package com.puppycrawl.tools.checkstyle.api; package com.puppycrawl.tools.checkstyle.api;


import java.util.TreeSet; import java.util.SortedSet;


/** /**
* Used by FileSetChecks to distribute AuditEvents to AuditListeners. * Used by FileSetChecks to distribute AuditEvents to AuditListeners.
Expand All @@ -43,5 +43,5 @@ public interface MessageDispatcher
* @param aFileName the audited file * @param aFileName the audited file
* @param aErrors the audit errors from the file * @param aErrors the audit errors from the file
*/ */
void fireErrors(String aFileName, TreeSet<LocalizedMessage> aErrors); void fireErrors(String aFileName, SortedSet<LocalizedMessage> aErrors);
} }
Expand Up @@ -101,13 +101,13 @@ public void visitToken(DetailAST aAST)
aAST.getText()); aAST.getText());
} }
} }
else if (getAbstractOption() == BlockOption.TEXT) { else if (getAbstractOption() == BlockOption.TEXT
if (!hasText(slistAST)) { && !hasText(slistAST))
log(slistAST.getLineNo(), {
slistAST.getColumnNo(), log(slistAST.getLineNo(),
"block.empty", slistAST.getColumnNo(),
aAST.getText()); "block.empty",
} aAST.getText());
} }
} }
} }
Expand Down
Expand Up @@ -69,14 +69,14 @@ public void visitToken(DetailAST aAST)
if (objBlock != null) { if (objBlock != null) {
DetailAST child = objBlock.getFirstChild(); DetailAST child = objBlock.getFirstChild();
while (child != null) { while (child != null) {
if (child.getType() == TokenTypes.METHOD_DEF) { if (child.getType() == TokenTypes.METHOD_DEF
if (CheckUtils.isEqualsMethod(child)) { && CheckUtils.isEqualsMethod(child))
if (hasObjectParameter(child)) { {
hasEqualsObject = true; if (hasObjectParameter(child)) {
} hasEqualsObject = true;
else { }
mEqualsMethods.add(child); else {
} mEqualsMethods.add(child);
} }
} }
child = child.getNextSibling(); child = child.getNextSibling();
Expand Down
Expand Up @@ -123,15 +123,18 @@ public void visitToken(final DetailAST aMethodCall)
final DetailAST expr = dot.getNextSibling().getFirstChild(); final DetailAST expr = dot.getNextSibling().getFirstChild();


if ("equals".equals(method.getText()) if ("equals".equals(method.getText())
|| (!mIgnoreEqualsIgnoreCase && "equalsIgnoreCase" && containsOneArg(expr) && containsAllSafeTokens(expr))
.equals(method.getText())))
{ {
if (containsOneArg(expr) && containsAllSafeTokens(expr)) { log(aMethodCall.getLineNo(), aMethodCall.getColumnNo(),
log(aMethodCall.getLineNo(), aMethodCall.getColumnNo(), "equals.avoid.null");
"equals".equals(method.getText()) }
? "equals.avoid.null"
: "equalsIgnoreCase.avoid.null"); if (!mIgnoreEqualsIgnoreCase
} && "equalsIgnoreCase".equals(method.getText())
&& containsOneArg(expr) && containsAllSafeTokens(expr))
{
log(aMethodCall.getLineNo(), aMethodCall.getColumnNo(),
"equalsIgnoreCase.avoid.null");
} }
} }


Expand Down
Expand Up @@ -133,14 +133,14 @@ public void visitToken(DetailAST aAST)


final DetailAST slist = aAST.findFirstToken(TokenTypes.SLIST); final DetailAST slist = aAST.findFirstToken(TokenTypes.SLIST);


if (!isTerminated(slist, true, true)) { if (!isTerminated(slist, true, true)
if (!hasFallTruComment(aAST, nextGroup)) { && !hasFallTruComment(aAST, nextGroup))
if (!isLastGroup) { {
log(nextGroup, "fall.through"); if (!isLastGroup) {
} log(nextGroup, "fall.through");
else { }
log(aAST, "fall.through.last"); else {
} log(aAST, "fall.through.last");
} }
} }
} }
Expand Down
Expand Up @@ -99,10 +99,11 @@ public void leaveToken(DetailAST aAst)
private void visitExpr(DetailAST aAst) private void visitExpr(DetailAST aAst)
{ {
mExprDepth++; mExprDepth++;
if (mExprDepth == 1) { if (mExprDepth == 1
if (!mInForHeader && (mLastStatementEnd == aAst.getLineNo())) { && !mInForHeader
log(aAst, "multiple.statements.line"); && (mLastStatementEnd == aAst.getLineNo()))
} {
log(aAst, "multiple.statements.line");
} }
} }


Expand Down
Expand Up @@ -134,11 +134,9 @@ private void checkException(FullIdent aExc, List<ClassInfo> aKnownExcs)
final ClassInfo newClassInfo = final ClassInfo newClassInfo =
createClassInfo(new Token(aExc), getCurrentClassName()); createClassInfo(new Token(aExc), getCurrentClassName());


if (!mAllowUnchecked) { if (!mAllowUnchecked && isUnchecked(newClassInfo.getClazz())) {
if (isUnchecked(newClassInfo.getClazz())) { log(aExc.getLineNo(), aExc.getColumnNo(),
log(aExc.getLineNo(), aExc.getColumnNo(), "redundant.throws.unchecked", aExc.getText());
"redundant.throws.unchecked", aExc.getText());
}
} }


boolean shouldAdd = true; boolean shouldAdd = true;
Expand Down
Expand Up @@ -430,11 +430,8 @@ private void findDuplicateFromLine(
} }


final Collection<Integer> ignoreEntries = aIgnore.get(aILine); final Collection<Integer> ignoreEntries = aIgnore.get(aILine);
// avoid Integer constructor whenever we can if (ignoreEntries != null && ignoreEntries.contains(jLine)) {
if (ignoreEntries != null) { continue;
if (ignoreEntries.contains(jLine)) {
continue;
}
} }


final int duplicateLines = final int duplicateLines =
Expand Down
Expand Up @@ -89,12 +89,12 @@ public void startElement(final String aNamespaceURI,
mStack.push(new PkgControl(pkg)); mStack.push(new PkgControl(pkg));
} }
else if ("subpackage".equals(aQName)) { else if ("subpackage".equals(aQName)) {
assert mStack.size() > 0; assert !mStack.isEmpty();
final String name = safeGet(aAtts, "name"); final String name = safeGet(aAtts, "name");
mStack.push(new PkgControl(mStack.peek(), name)); mStack.push(new PkgControl(mStack.peek(), name));
} }
else if ("allow".equals(aQName) || "disallow".equals(aQName)) { else if ("allow".equals(aQName) || "disallow".equals(aQName)) {
assert mStack.size() > 0; assert !mStack.isEmpty();
// Need to handle either "pkg" or "class" attribute. // Need to handle either "pkg" or "class" attribute.
// May have "exact-match" for "pkg" // May have "exact-match" for "pkg"
// May have "local-only" // May have "local-only"
Expand Down
Expand Up @@ -294,8 +294,7 @@ private void doVisitToken(FullIdent aIdent, boolean aIsStatic,
} }
} }
else if (groupIdx == mLastGroup) { else if (groupIdx == mLastGroup) {
doVisitTokenInSameGroup(aIdent, aIsStatic, aPrevious, name, doVisitTokenInSameGroup(aIsStatic, aPrevious, name, line);
line);
} }
else { else {
log(line, "import.ordering", name); log(line, "import.ordering", name);
Expand All @@ -309,14 +308,13 @@ else if (groupIdx == mLastGroup) {
/** /**
* Shares processing... * Shares processing...
* *
* @param aIdent the import to process.
* @param aIsStatic whether the token is static or not. * @param aIsStatic whether the token is static or not.
* @param aPrevious previous non-static but current is static (above), or * @param aPrevious previous non-static but current is static (above), or
* previous static but current is non-static (under). * previous static but current is non-static (under).
* @param aName the name of the current import. * @param aName the name of the current import.
* @param aLine the line 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) boolean aPrevious, String aName, int aLine)
{ {
if (!mOrdered) { if (!mOrdered) {
Expand Down
Expand Up @@ -260,7 +260,7 @@ protected final void processAST(DetailAST aAST)
} }
} }
else { else {
checkComment(aAST, cmt, theScope); checkComment(aAST, cmt);
} }
} }
} }
Expand Down Expand Up @@ -313,13 +313,12 @@ private boolean shouldCheck(final DetailAST aAST, final Scope aScope)
* *
* @param aAST the token for the method * @param aAST the token for the method
* @param aComment the Javadoc comment * @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<JavadocTag> tags = getMethodTags(aComment); final List<JavadocTag> tags = getMethodTags(aComment);


if (hasShortCircuitTag(aAST, tags, aScope)) { if (hasShortCircuitTag(aAST, tags)) {
return; return;
} }


Expand Down Expand Up @@ -354,11 +353,10 @@ private void checkComment(DetailAST aAST, TextBlock aComment, Scope aScope)
* *
* @param aAST the construct being checked * @param aAST the construct being checked
* @param aTags the list of Javadoc tags associated with the construct * @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. * @return true if the construct has a short circuit tag.
*/ */
private boolean hasShortCircuitTag(final DetailAST aAST, private boolean hasShortCircuitTag(final DetailAST aAST,
final List<JavadocTag> aTags, final Scope aScope) final List<JavadocTag> aTags)
{ {
// Check if it contains {@inheritDoc} tag // Check if it contains {@inheritDoc} tag
if ((aTags.size() != 1) if ((aTags.size() != 1)
Expand Down
Expand Up @@ -74,7 +74,7 @@ public HtmlTag nextTag()
*/ */
public boolean hasNextTag() public boolean hasNextTag()
{ {
return (mTags.size() > 0); return !mTags.isEmpty();
} }


/** /**
Expand Down
Expand Up @@ -91,14 +91,13 @@ public void visitToken(DetailAST aAST)
processLeft(theAst); processLeft(theAst);
} }
} }
else if ((theAst.getParent() == null) else if (((theAst.getParent() == null)
|| (theAst.getParent().getType() != TokenTypes.TYPECAST) || (theAst.getParent().getType() != TokenTypes.TYPECAST)
|| (theAst.getParent().findFirstToken(TokenTypes.RPAREN) || (theAst.getParent().findFirstToken(TokenTypes.RPAREN)
!= theAst)) != theAst))
&& !isFollowsEmptyForIterator(theAst))
{ {
if (!isFollowsEmptyForIterator(theAst)) { processRight(theAst);
processRight(theAst);
}
} }
} }


Expand Down
Expand Up @@ -154,18 +154,12 @@ public boolean accept(AuditEvent aEvent)
return false; return false;
} }


// reject if line matches a line CSV value. if (mLineFilter != null && mLineFilter.accept(aEvent.getLine())) {
if (mLineFilter != null) { return false;
if (mLineFilter.accept(aEvent.getLine())) {
return false;
}
} }


// reject if column matches a column CSV value. if (mColumnFilter != null && mColumnFilter.accept(aEvent.getColumn())) {
if (mColumnFilter != null) { return false;
if (mColumnFilter.accept(aEvent.getColumn())) {
return false;
}
} }
return true; return true;
} }
Expand Down
Expand Up @@ -484,9 +484,9 @@ private void tagSuppressions(Collection<TextBlock> aComments)
for (final TextBlock comment : aComments) { for (final TextBlock comment : aComments) {
final int startLineNo = comment.getStartLineNo(); final int startLineNo = comment.getStartLineNo();
final String[] text = comment.getText(); final String[] text = comment.getText();
tagCommentLine(text[0], startLineNo, comment.getStartColNo()); tagCommentLine(text[0], startLineNo);
for (int i = 1; i < text.length; i++) { for (int i = 1; i < text.length; i++) {
tagCommentLine(text[i], startLineNo + i, 0); tagCommentLine(text[i], startLineNo + i);
} }
} }
} }
Expand All @@ -496,9 +496,8 @@ private void tagSuppressions(Collection<TextBlock> aComments)
* checkstyle reporting on or the format for turning reporting off. * checkstyle reporting on or the format for turning reporting off.
* @param aText the string to tag. * @param aText the string to tag.
* @param aLine the line number of aText. * @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); final Matcher matcher = mCommentRegexp.matcher(aText);
if (matcher.find()) { if (matcher.find()) {
Expand Down

0 comments on commit 1d614c3

Please sign in to comment.