From 88830ca708e2deb22cae333057ebddd15f7f9c57 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Wed, 7 Oct 2015 06:35:04 -0700 Subject: [PATCH] Issue #974: PMD violation ConfusingTernary (partial fix) --- .../indentation/BlockParentHandler.java | 16 ++--- .../checks/indentation/ForHandler.java | 18 ++--- .../checkstyle/checks/javadoc/TagParser.java | 66 +++++++++++-------- .../checks/javadoc/WriteTagCheck.java | 6 +- .../checkstyle/filters/SuppressElement.java | 12 ++-- .../SuppressWithNearbyCommentFilter.java | 8 +-- .../filters/SuppressionCommentFilter.java | 34 +++++----- 7 files changed, 86 insertions(+), 74 deletions(-) diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/BlockParentHandler.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/BlockParentHandler.java index 35626478d..3d500f7be 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/BlockParentHandler.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/BlockParentHandler.java @@ -270,19 +270,19 @@ public class BlockParentHandler extends AbstractExpressionHandler { checkRCurly(); } final DetailAST listChild = getListChild(); - if (listChild != null) { + if (listChild == null) { + checkNonListChild(); + } + else { // NOTE: switch statements usually don't have curlies if (!hasCurlies() || !areOnSameLine(getLCurly(), getRCurly())) { checkChildren(listChild, - getCheckedChildren(), - getChildrenExpectedLevel(), - true, - canChildrenBeNested()); + getCheckedChildren(), + getChildrenExpectedLevel(), + true, + canChildrenBeNested()); } } - else { - checkNonListChild(); - } } /** diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/ForHandler.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/ForHandler.java index b13a495a5..d94e29b56 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/ForHandler.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/ForHandler.java @@ -49,23 +49,23 @@ public class ForHandler extends BlockParentHandler { new IndentLevel(getLevel(), getBasicOffset()); final DetailAST init = getMainAst().findFirstToken(TokenTypes.FOR_INIT); - if (init != null) { + if (init == null) { + // for each + final DetailAST forEach = + getMainAst().findFirstToken(TokenTypes.FOR_EACH_CLAUSE); + checkExpressionSubtree(forEach, expected, false, false); + } + else { checkExpressionSubtree(init, expected, false, false); final DetailAST cond = - getMainAst().findFirstToken(TokenTypes.FOR_CONDITION); + getMainAst().findFirstToken(TokenTypes.FOR_CONDITION); checkExpressionSubtree(cond, expected, false, false); final DetailAST forIterator = - getMainAst().findFirstToken(TokenTypes.FOR_ITERATOR); + getMainAst().findFirstToken(TokenTypes.FOR_ITERATOR); checkExpressionSubtree(forIterator, expected, false, false); } - // for each - else { - final DetailAST forEach = - getMainAst().findFirstToken(TokenTypes.FOR_EACH_CLAUSE); - checkExpressionSubtree(forEach, expected, false, false); - } } @Override diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java index 7e263feaf..ea783af3a 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java @@ -101,40 +101,52 @@ class TagParser { if (isCommentTag(text, position)) { position = skipHtmlComment(text, position); } - else if (!isTag(text, position)) { - position = getNextCharPos(text, position); + else if (isTag(text, position)) { + position = parseTag(text, lineNo, nLines, position); } else { - // find end of tag - final Point endTag = findChar(text, '>', position); - final boolean incompleteTag = endTag.getLineNo() >= nLines; - // get tag id (one word) - final String tagId; - - if (incompleteTag) { - tagId = ""; - } - else { - tagId = getTagId(text, position); - } - // is this closed tag - final boolean closedTag = - endTag.getLineNo() < nLines - && text[endTag.getLineNo()] - .charAt(endTag.getColumnNo() - 1) == '/'; - // add new tag - add(new HtmlTag(tagId, - position.getLineNo() + lineNo, - position.getColumnNo(), - closedTag, - incompleteTag, - text[position.getLineNo()])); - position = endTag; + position = getNextCharPos(text, position); } position = findChar(text, '<', position); } } + /** + * Parses the tag and return position after it + * @param text the source line to parse. + * @param lineNo the source line number. + * @param nLines line length + * @param position start position for parsing + * @return position after tag + */ + private Point parseTag(String[] text, int lineNo, final int nLines, Point position) { + // find end of tag + final Point endTag = findChar(text, '>', position); + final boolean incompleteTag = endTag.getLineNo() >= nLines; + // get tag id (one word) + final String tagId; + + if (incompleteTag) { + tagId = ""; + } + else { + tagId = getTagId(text, position); + } + // is this closed tag + final boolean closedTag = + endTag.getLineNo() < nLines + && text[endTag.getLineNo()] + .charAt(endTag.getColumnNo() - 1) == '/'; + // add new tag + add(new HtmlTag(tagId, + position.getLineNo() + lineNo, + position.getColumnNo(), + closedTag, + incompleteTag, + text[position.getLineNo()])); + return endTag; + } + /** * Checks if the given position is start one for HTML tag. * @param javadocText text of javadoc comments. diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java index 691cd0744..50803baea 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/WriteTagCheck.java @@ -186,11 +186,11 @@ public class WriteTagCheck tagCount += 1; final int contentStart = matcher.start(1); final String content = commentValue.substring(contentStart); - if (tagFormatRE != null && !tagFormatRE.matcher(content).find()) { - log(lineNo + i - comment.length, TAG_FORMAT, tag, tagFormat); + if (tagFormatRE == null || tagFormatRE.matcher(content).find()) { + logTag(lineNo + i - comment.length, tag, content); } else { - logTag(lineNo + i - comment.length, tag, content); + log(lineNo + i - comment.length, TAG_FORMAT, tag, tagFormat); } } } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java index d20956a89..8d75aa4eb 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java @@ -102,11 +102,11 @@ public class SuppressElement */ public void setLines(String lines) { linesCSV = lines; - if (lines != null) { - lineFilter = new CSVFilter(lines); + if (lines == null) { + lineFilter = null; } else { - lineFilter = null; + lineFilter = new CSVFilter(lines); } } @@ -117,11 +117,11 @@ public class SuppressElement */ public void setColumns(String columns) { columnsCSV = columns; - if (columns != null) { - columnFilter = new CSVFilter(columns); + if (columns == null) { + columnFilter = null; } else { - columnFilter = null; + columnFilter = new CSVFilter(columns); } } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java index 21788909d..a83e705b0 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java @@ -322,14 +322,14 @@ public class SuppressWithNearbyCommentFilter try { format = expandFromComment(text, filter.checkFormat, filter.commentRegexp); tagCheckRegexp = Pattern.compile(format); - if (filter.messageFormat != null) { + if (filter.messageFormat == null) { + tagMessageRegexp = null; + } + else { format = expandFromComment( text, filter.messageFormat, filter.commentRegexp); tagMessageRegexp = Pattern.compile(format); } - else { - tagMessageRegexp = null; - } format = expandFromComment( text, filter.influenceFormat, filter.commentRegexp); int influence; diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java index 523ff5ae2..8373be989 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java @@ -340,29 +340,29 @@ public class SuppressionCommentFilter format = expandFromComment(text, filter.checkFormat, filter.onRegexp); tagCheckRegexp = Pattern.compile(format); - if (filter.messageFormat != null) { - format = - expandFromComment(text, filter.messageFormat, filter.onRegexp); - tagMessageRegexp = Pattern.compile(format); + if (filter.messageFormat == null) { + tagMessageRegexp = null; } else { - tagMessageRegexp = null; + format = + expandFromComment(text, filter.messageFormat, filter.onRegexp); + tagMessageRegexp = Pattern.compile(format); } } else { format = expandFromComment(text, filter.checkFormat, filter.offRegexp); tagCheckRegexp = Pattern.compile(format); - if (filter.messageFormat != null) { - format = - expandFromComment( - text, - filter.messageFormat, - filter.offRegexp); - tagMessageRegexp = Pattern.compile(format); + if (filter.messageFormat == null) { + tagMessageRegexp = null; } else { - tagMessageRegexp = null; + format = + expandFromComment( + text, + filter.messageFormat, + filter.offRegexp); + tagMessageRegexp = Pattern.compile(format); } } } @@ -446,12 +446,12 @@ public class SuppressionCommentFilter boolean match = false; final Matcher tagMatcher = tagCheckRegexp.matcher(event.getSourceName()); if (tagMatcher.find()) { - if (tagMessageRegexp != null) { - final Matcher messageMatcher = tagMessageRegexp.matcher(event.getMessage()); - match = messageMatcher.find(); + if (tagMessageRegexp == null) { + match = true; } else { - match = true; + final Matcher messageMatcher = tagMessageRegexp.matcher(event.getMessage()); + match = messageMatcher.find(); } } return match;