diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/AbstractNcssCountRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/AbstractNcssCountRule.java index 5cb8880a79..9679202daa 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/AbstractNcssCountRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/AbstractNcssCountRule.java @@ -81,7 +81,7 @@ public abstract class AbstractNcssCountRule extends AbstractStatisticalApexRule * @return count of the number of children of the node, plus one */ protected Integer countNodeChildren(Node node, Object data) { - Integer nodeCount = null; + Integer nodeCount; int lineCount = 0; for (int i = 0; i < node.jjtGetNumChildren(); i++) { nodeCount = (Integer) ((ApexNode) node.jjtGetChild(i)).jjtAccept(this, data); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/TooManyFieldsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/TooManyFieldsRule.java index 5d5b671b86..3d213d0233 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/TooManyFieldsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/complexity/TooManyFieldsRule.java @@ -53,9 +53,9 @@ public class TooManyFieldsRule extends AbstractApexRule { bumpCounterFor(clazz); } } - for (String k : stats.keySet()) { - int val = stats.get(k); - Node n = nodes.get(k); + for (Map.Entry entry : stats.entrySet()) { + int val = entry.getValue(); + Node n = nodes.get(entry.getKey()); if (val > maxFields) { addViolation(data, n); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PropertyDescriptorFields.java b/pmd-core/src/main/java/net/sourceforge/pmd/PropertyDescriptorFields.java index 1a95dac1ee..f36e773c77 100755 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PropertyDescriptorFields.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PropertyDescriptorFields.java @@ -36,4 +36,6 @@ public class PropertyDescriptorFields { public static final String MAX = "max"; /** To limit the range of valid values, e.g. to Enums */ public static final String LEGAL_PACKAGES = "legalPackages"; + + private PropertyDescriptorFields() {} } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java index 45de9c8fec..f4ce9379f1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSet.java @@ -34,6 +34,7 @@ import net.sourceforge.pmd.util.filter.Filters; public class RuleSet { private static final Logger LOG = Logger.getLogger(RuleSet.class.getName()); + private static final String MISSING_RULE = "Missing rule"; private List rules = new ArrayList<>(); private String fileName; @@ -80,7 +81,7 @@ public class RuleSet { */ public void addRule(Rule rule) { if (rule == null) { - throw new IllegalArgumentException("Missing rule"); + throw new IllegalArgumentException(MISSING_RULE); } rules.add(rule); } @@ -96,7 +97,7 @@ public class RuleSet { */ public boolean addRuleReplaceIfExists(Rule rule) { if (rule == null) { - throw new IllegalArgumentException("Missing rule"); + throw new IllegalArgumentException(MISSING_RULE); } boolean replaced = false; @@ -122,7 +123,7 @@ public class RuleSet { */ public boolean addRuleIfNotExists(Rule rule) { if (rule == null) { - throw new IllegalArgumentException("Missing rule"); + throw new IllegalArgumentException(MISSING_RULE); } boolean exists = false; @@ -182,10 +183,8 @@ public class RuleSet { */ public boolean usesDFA(Language language) { for (Rule r : rules) { - if (r.getLanguage().equals(language)) { - if (r.usesDFA()) { - return true; - } + if (r.getLanguage().equals(language) && r.usesDFA()) { + return true; } } return false; @@ -509,10 +508,8 @@ public class RuleSet { */ public boolean usesTypeResolution(Language language) { for (Rule r : rules) { - if (r.getLanguage().equals(language)) { - if (r.usesTypeResolution()) { - return true; - } + if (r.getLanguage().equals(language) && r.usesTypeResolution()) { + return true; } } return false; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index 333edf58b6..81bc00cfa1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -48,6 +48,13 @@ public class RuleSetFactory { private static final Logger LOG = Logger.getLogger(RuleSetFactory.class.getName()); + private static final String DESCRIPTION = "description"; + private static final String UNEXPECTED_ELEMENT = "Unexpected element <"; + private static final String PRIORITY = "priority"; + private static final String FOR_RULE = "' for Rule "; + private static final String MESSAGE = "message"; + private static final String EXTERNAL_INFO_URL = "externalInfoUrl"; + private ClassLoader classLoader = RuleSetFactory.class.getClassLoader(); private RulePriority minimumPriority = RulePriority.LOW; private boolean warnDeprecated = false; @@ -257,7 +264,7 @@ public class RuleSetFactory { Node node = nodeList.item(i); if (node.getNodeType() == Node.ELEMENT_NODE) { String nodeName = node.getNodeName(); - if ("description".equals(nodeName)) { + if (DESCRIPTION.equals(nodeName)) { ruleSet.setDescription(parseTextNode(node)); } else if ("include-pattern".equals(nodeName)) { ruleSet.addIncludePattern(parseTextNode(node)); @@ -266,7 +273,7 @@ public class RuleSetFactory { } else if ("rule".equals(nodeName)) { parseRuleNode(ruleSetReferenceId, ruleSet, node, withDeprecatedRuleReferences); } else { - throw new IllegalArgumentException("Unexpected element <" + node.getNodeName() + throw new IllegalArgumentException(UNEXPECTED_ELEMENT + node.getNodeName() + "> encountered as child of element."); } } @@ -346,7 +353,7 @@ public class RuleSetFactory { String excludedRuleName = excludeElement.getAttribute("name"); ruleSetReference.addExclude(excludedRuleName); excludedRulesCheck.add(excludedRuleName); - } else if (isElementNode(child, "priority")) { + } else if (isElementNode(child, PRIORITY)) { priority = parseTextNode(child).trim(); } } @@ -406,7 +413,7 @@ public class RuleSetFactory { String languageName = ruleElement.getAttribute("language"); Language language = LanguageRegistry.findLanguageByTerseName(languageName); if (language == null) { - throw new IllegalArgumentException("Unknown Language '" + languageName + "' for Rule " + rule.getName() + throw new IllegalArgumentException("Unknown Language '" + languageName + FOR_RULE + rule.getName() + ", supported Languages are " + LanguageRegistry.commaSeparatedTerseNamesForLanguage(LanguageRegistry.findWithRuleSupport())); } @@ -424,7 +431,7 @@ public class RuleSetFactory { LanguageVersion minimumLanguageVersion = language.getVersion(minimumLanguageVersionName); if (minimumLanguageVersion == null) { throw new IllegalArgumentException("Unknown minimum Language Version '" + minimumLanguageVersionName - + "' for Language '" + language.getTerseName() + "' for Rule " + rule.getName() + + "' for Language '" + language.getTerseName() + FOR_RULE + rule.getName() + "; supported Language Versions are: " + LanguageRegistry.commaSeparatedTerseNamesForLanguageVersion(language.getVersions())); } @@ -436,7 +443,7 @@ public class RuleSetFactory { LanguageVersion maximumLanguageVersion = language.getVersion(maximumLanguageVersionName); if (maximumLanguageVersion == null) { throw new IllegalArgumentException("Unknown maximum Language Version '" + maximumLanguageVersionName - + "' for Language '" + language.getTerseName() + "' for Rule " + rule.getName() + + "' for Language '" + language.getTerseName() + FOR_RULE + rule.getName() + "; supported Language Versions are: " + LanguageRegistry.commaSeparatedTerseNamesForLanguageVersion(language.getVersions())); } @@ -447,7 +454,7 @@ public class RuleSetFactory { throw new IllegalArgumentException("The minimum Language Version '" + rule.getMinimumLanguageVersion().getTerseName() + "' must be prior to the maximum Language Version '" - + rule.getMaximumLanguageVersion().getTerseName() + "' for Rule " + rule.getName() + + rule.getMaximumLanguageVersion().getTerseName() + FOR_RULE + rule.getName() + "; perhaps swap them around?"); } @@ -455,9 +462,9 @@ public class RuleSetFactory { if (StringUtil.isNotEmpty(since)) { rule.setSince(since); } - rule.setMessage(ruleElement.getAttribute("message")); + rule.setMessage(ruleElement.getAttribute(MESSAGE)); rule.setRuleSetName(ruleSet.getName()); - rule.setExternalInfoUrl(ruleElement.getAttribute("externalInfoUrl")); + rule.setExternalInfoUrl(ruleElement.getAttribute(EXTERNAL_INFO_URL)); if (hasAttributeSetTrue(ruleElement, "dfa")) { rule.setUsesDFA(); @@ -474,16 +481,16 @@ public class RuleSetFactory { continue; } String nodeName = node.getNodeName(); - if (nodeName.equals("description")) { + if (nodeName.equals(DESCRIPTION)) { rule.setDescription(parseTextNode(node)); } else if (nodeName.equals("example")) { rule.addExample(parseTextNode(node)); - } else if (nodeName.equals("priority")) { + } else if (nodeName.equals(PRIORITY)) { rule.setPriority(RulePriority.valueOf(Integer.parseInt(parseTextNode(node).trim()))); } else if (nodeName.equals("properties")) { parsePropertiesNode(rule, node); } else { - throw new IllegalArgumentException("Unexpected element <" + nodeName + throw new IllegalArgumentException(UNEXPECTED_ELEMENT + nodeName + "> encountered as child of element for Rule " + rule.getName()); } @@ -581,25 +588,25 @@ public class RuleSetFactory { if (ruleElement.hasAttribute("name")) { ruleReference.setName(ruleElement.getAttribute("name")); } - if (ruleElement.hasAttribute("message")) { - ruleReference.setMessage(ruleElement.getAttribute("message")); + if (ruleElement.hasAttribute(MESSAGE)) { + ruleReference.setMessage(ruleElement.getAttribute(MESSAGE)); } - if (ruleElement.hasAttribute("externalInfoUrl")) { - ruleReference.setExternalInfoUrl(ruleElement.getAttribute("externalInfoUrl")); + if (ruleElement.hasAttribute(EXTERNAL_INFO_URL)) { + ruleReference.setExternalInfoUrl(ruleElement.getAttribute(EXTERNAL_INFO_URL)); } for (int i = 0; i < ruleElement.getChildNodes().getLength(); i++) { Node node = ruleElement.getChildNodes().item(i); if (node.getNodeType() == Node.ELEMENT_NODE) { - if (node.getNodeName().equals("description")) { + if (node.getNodeName().equals(DESCRIPTION)) { ruleReference.setDescription(parseTextNode(node)); } else if (node.getNodeName().equals("example")) { ruleReference.addExample(parseTextNode(node)); - } else if (node.getNodeName().equals("priority")) { + } else if (node.getNodeName().equals(PRIORITY)) { ruleReference.setPriority(RulePriority.valueOf(Integer.parseInt(parseTextNode(node)))); } else if (node.getNodeName().equals("properties")) { parsePropertiesNode(ruleReference, node); } else { - throw new IllegalArgumentException("Unexpected element <" + node.getNodeName() + throw new IllegalArgumentException(UNEXPECTED_ELEMENT + node.getNodeName() + "> encountered as child of element for Rule " + ruleReference.getName()); } } @@ -630,11 +637,9 @@ public class RuleSetFactory { NodeList rules = ruleSetElement.getElementsByTagName("rule"); for (int i = 0; i < rules.getLength(); i++) { Element rule = (Element) rules.item(i); - if (rule.hasAttribute("name")) { - if (rule.getAttribute("name").equals(ruleName)) { - found = true; - break; - } + if (rule.hasAttribute("name") && rule.getAttribute("name").equals(ruleName)) { + found = true; + break; } } } catch (Exception e) { diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 4c7573c5d5..20b0347e8c 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -47,6 +47,7 @@ you'll need a java8 runtime environment. * [#31](https://github.com/adangel/pmd/pull/31): Added file encoding detection to CPD. * [#32](https://github.com/adangel/pmd/pull/32): Extended Objective-C grammar to accept UTF-8 escapes (\uXXXX) in string literals. * [#33](https://github.com/adangel/pmd/pull/33): Added support for Swift to CPD. +* [#34](https://github.com/adangel/pmd/pull/34): multiple code improvements: squid:S1192, squid:S1118, squid:S1066, squid:S1854, squid:S2864 * [#72](https://github.com/pmd/pmd/pull/72): Added capability in Java and JSP parser for tracking tokens. * [#73](https://github.com/pmd/pmd/pull/73): Add rule to look for invalid message format in slf4j loggers * [#74](https://github.com/pmd/pmd/pull/74): Fix rendering CommentDefaultAccessModifier description as code