diff --git a/docs/pages/pmd/rules/ecmascript.md b/docs/pages/pmd/rules/ecmascript.md index 84bbba4703..aea2b60be7 100644 --- a/docs/pages/pmd/rules/ecmascript.md +++ b/docs/pages/pmd/rules/ecmascript.md @@ -5,33 +5,30 @@ folder: pmd/rules --- List of rulesets and rules contained in each ruleset. -* [Basic Ecmascript](pmd_rules_ecmascript_basic.html): Rules concerning basic ECMAScript guidelines. -* [Braces](pmd_rules_ecmascript_braces.html): The Braces Ruleset contains a collection of braces rules. -* [Controversial Ecmascript](pmd_rules_ecmascript_controversial.html): The Controversial ruleset contains rules that, for whatever reason, are considered controversial. They are held here to allow people to include them as they see fit within their custom rulesets. -* [Unnecessary](pmd_rules_ecmascript_unnecessary.html): The Unnecessary Ruleset contains a collection of rules for unnecessary code. +* [Best Practices](pmd_rules_ecmascript_bestpractices.html): +* [Code Style](pmd_rules_ecmascript_codestyle.html): +* [Error Prone](pmd_rules_ecmascript_errorprone.html): -## Basic Ecmascript -* [AssignmentInOperand](pmd_rules_ecmascript_basic.html#assignmentinoperand): Avoid assignments in operands; this can make code more complicated and harder to read. This is s... -* [AvoidTrailingComma](pmd_rules_ecmascript_basic.html#avoidtrailingcomma): This rule helps improve code portability due to differences in browser treatment of trailing comm... -* [ConsistentReturn](pmd_rules_ecmascript_basic.html#consistentreturn): ECMAScript does provide for return types on functions, and therefore there is no solid rule as to... -* [EqualComparison](pmd_rules_ecmascript_basic.html#equalcomparison): Using == in condition may lead to unexpected results, as the variables are automatically casted t... -* [GlobalVariable](pmd_rules_ecmascript_basic.html#globalvariable): This rule helps to avoid using accidently global variables by simply missing the "var" declaratio... -* [InnaccurateNumericLiteral](pmd_rules_ecmascript_basic.html#innaccuratenumericliteral): The numeric literal will have at different value at runtime, which can happen if you provide too ... -* [ScopeForInVariable](pmd_rules_ecmascript_basic.html#scopeforinvariable): A for-in loop in which the variable name is not explicitly scoped to the enclosing scope with the... -* [UnreachableCode](pmd_rules_ecmascript_basic.html#unreachablecode): A 'return', 'break', 'continue', or 'throw' statement should be the last in a block. Statements a... -* [UseBaseWithParseInt](pmd_rules_ecmascript_basic.html#usebasewithparseint): This rule checks for usages of parseInt. While the second parameter is optional and usually defau... +## Best Practices +* [AvoidWithStatement](pmd_rules_ecmascript_bestpractices.html#avoidwithstatement): Avoid using with - it's bad news +* [ConsistentReturn](pmd_rules_ecmascript_bestpractices.html#consistentreturn): ECMAScript does provide for return types on functions, and therefore there is no solid rule as to... +* [GlobalVariable](pmd_rules_ecmascript_bestpractices.html#globalvariable): This rule helps to avoid using accidently global variables by simply missing the "var" declaratio... +* [ScopeForInVariable](pmd_rules_ecmascript_bestpractices.html#scopeforinvariable): A for-in loop in which the variable name is not explicitly scoped to the enclosing scope with the... +* [UseBaseWithParseInt](pmd_rules_ecmascript_bestpractices.html#usebasewithparseint): This rule checks for usages of parseInt. While the second parameter is optional and usually defau... -## Braces -* [ForLoopsMustUseBraces](pmd_rules_ecmascript_braces.html#forloopsmustusebraces): Avoid using 'for' statements without using curly braces. -* [IfElseStmtsMustUseBraces](pmd_rules_ecmascript_braces.html#ifelsestmtsmustusebraces): Avoid using if..else statements without using curly braces. -* [IfStmtsMustUseBraces](pmd_rules_ecmascript_braces.html#ifstmtsmustusebraces): Avoid using if statements without using curly braces. -* [WhileLoopsMustUseBraces](pmd_rules_ecmascript_braces.html#whileloopsmustusebraces): Avoid using 'while' statements without using curly braces. +## Code Style +* [AssignmentInOperand](pmd_rules_ecmascript_codestyle.html#assignmentinoperand): Avoid assignments in operands; this can make code more complicated and harder to read. This is s... +* [AvoidTrailingComma](pmd_rules_ecmascript_codestyle.html#avoidtrailingcomma): This rule helps improve code portability due to differences in browser treatment of trailing comm... +* [ForLoopsMustUseBraces](pmd_rules_ecmascript_codestyle.html#forloopsmustusebraces): Avoid using 'for' statements without using curly braces. +* [IfElseStmtsMustUseBraces](pmd_rules_ecmascript_codestyle.html#ifelsestmtsmustusebraces): Avoid using if..else statements without using curly braces. +* [IfStmtsMustUseBraces](pmd_rules_ecmascript_codestyle.html#ifstmtsmustusebraces): Avoid using if statements without using curly braces. +* [NoElseReturn](pmd_rules_ecmascript_codestyle.html#noelsereturn): The else block in a if-else-construct is unnecessary if the 'if' block contains a return.Then the... +* [UnnecessaryBlock](pmd_rules_ecmascript_codestyle.html#unnecessaryblock): An unnecessary Block is present. Such Blocks are often used in other languages tointroduce a new... +* [UnnecessaryParentheses](pmd_rules_ecmascript_codestyle.html#unnecessaryparentheses): Unnecessary parentheses should be removed. +* [UnreachableCode](pmd_rules_ecmascript_codestyle.html#unreachablecode): A 'return', 'break', 'continue', or 'throw' statement should be the last in a block. Statements a... +* [WhileLoopsMustUseBraces](pmd_rules_ecmascript_codestyle.html#whileloopsmustusebraces): Avoid using 'while' statements without using curly braces. -## Controversial Ecmascript -* [AvoidWithStatement](pmd_rules_ecmascript_controversial.html#avoidwithstatement): Avoid using with - it's bad news - -## Unnecessary -* [NoElseReturn](pmd_rules_ecmascript_unnecessary.html#noelsereturn): The else block in a if-else-construct is unnecessary if the 'if' block contains a return.Then the... -* [UnnecessaryBlock](pmd_rules_ecmascript_unnecessary.html#unnecessaryblock): An unnecessary Block is present. Such Blocks are often used in other languages tointroduce a new... -* [UnnecessaryParentheses](pmd_rules_ecmascript_unnecessary.html#unnecessaryparentheses): Unnecessary parentheses should be removed. +## Error Prone +* [EqualComparison](pmd_rules_ecmascript_errorprone.html#equalcomparison): Using == in condition may lead to unexpected results, as the variables are automatically casted t... +* [InnaccurateNumericLiteral](pmd_rules_ecmascript_errorprone.html#innaccuratenumericliteral): The numeric literal will have a different value at runtime, which can happen if you provide too m... diff --git a/docs/pages/pmd/rules/ecmascript/basic.md b/docs/pages/pmd/rules/ecmascript/basic.md deleted file mode 100644 index 9663b6c9d0..0000000000 --- a/docs/pages/pmd/rules/ecmascript/basic.md +++ /dev/null @@ -1,378 +0,0 @@ ---- -title: Basic Ecmascript -summary: Rules concerning basic ECMAScript guidelines. -permalink: pmd_rules_ecmascript_basic.html -folder: pmd/rules/ecmascript -sidebaractiveurl: /pmd_rules_ecmascript.html -editmepath: ../pmd-javascript/src/main/resources/rulesets/ecmascript/basic.xml -keywords: Basic Ecmascript, AssignmentInOperand, UnreachableCode, InnaccurateNumericLiteral, ConsistentReturn, ScopeForInVariable, EqualComparison, GlobalVariable, AvoidTrailingComma, UseBaseWithParseInt ---- -## AssignmentInOperand - -**Since:** PMD 5.0 - -**Priority:** Medium High (2) - -Avoid assignments in operands; this can make code more complicated and harder to read. This is sometime -indicative of the bug where the assignment operator '=' was used instead of the equality operator '=='. - -``` -//IfStatement[$allowIf = "false"]/child::node()[1]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] -| - //WhileLoop[$allowWhile = "false"]/child::node()[1]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] -| - //DoLoop[$allowWhile = "false"]/child::node()[2]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] -| - //ForLoop[$allowFor = "false"]/child::node()[2]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] -| - //ConditionalExpression[$allowTernary = "false"]/child::node()[1]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] -| - //ConditionalExpression[$allowTernaryResults = "false"]/child::node()[position() = 2 or position() = 3]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] -``` - -**Example(s):** - -``` javascript -var x = 2; -// Bad -if ((x = getX()) == 3) { - alert('3!'); -} - -function getX() { - return 3; -} -``` - -**This rule has the following properties:** - -|Name|Default Value|Description| -|----|-------------|-----------| -|allowIf|false|Allow assignment within the conditional expression of an if statement| -|allowFor|false|Allow assignment within the conditional expression of a for statement| -|allowWhile|false|Allow assignment within the conditional expression of a while statement| -|allowTernary|false|Allow assignment within the conditional expression of a ternary operator| -|allowTernaryResults|false|Allow assignment within the result expressions of a ternary operator| -|allowIncrementDecrement|false|Allow increment or decrement operators within the conditional expression of an if, for, or while statement| - -**Use this rule by referencing it:** -``` xml - -``` - -## AvoidTrailingComma - -**Since:** PMD 5.1 - -**Priority:** High (1) - -This rule helps improve code portability due to differences in browser treatment of trailing commas in object or array literals. - -``` -//ObjectLiteral[$allowObjectLiteral = "false" and @TrailingComma = 'true'] -| -//ArrayLiteral[$allowArrayLiteral = "false" and @TrailingComma = 'true'] -``` - -**Example(s):** - -``` javascript -function(arg) { - var obj1 = { a : 1 }; // Ok - var arr1 = [ 1, 2 ]; // Ok - - var obj2 = { a : 1, }; // Syntax error in some browsers! - var arr2 = [ 1, 2, ]; // Length 2 or 3 depending on the browser! -} -``` - -**This rule has the following properties:** - -|Name|Default Value|Description| -|----|-------------|-----------| -|allowObjectLiteral|false|Allow a trailing comma within an object literal| -|allowArrayLiteral|false|Allow a trailing comma within an array literal| - -**Use this rule by referencing it:** -``` xml - -``` - -## ConsistentReturn - -**Since:** PMD 5.0 - -**Priority:** Medium High (2) - -ECMAScript does provide for return types on functions, and therefore there is no solid rule as to their usage. -However, when a function does use returns they should all have a value, or all with no value. Mixed return -usage is likely a bug, or at best poor style. - -**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.ecmascript.rule.basic.ConsistentReturnRule](https://github.com/pmd/pmd/blob/master/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/rule/basic/ConsistentReturnRule.java) - -**Example(s):** - -``` javascript -// Ok -function foo() { - if (condition1) { - return true; - } - return false; -} - -// Bad -function bar() { - if (condition1) { - return; - } - return false; -} -``` - -**This rule has the following properties:** - -|Name|Default Value|Description| -|----|-------------|-----------| -|rhinoLanguageVersion|VERSION_DEFAULT|Specifies the Rhino Language Version to use for parsing. Defaults to Rhino default.| -|recordingLocalJsDocComments|true|Specifies that JsDoc comments are produced in the AST.| -|recordingComments|true|Specifies that comments are produced in the AST.| - -**Use this rule by referencing it:** -``` xml - -``` - -## EqualComparison - -**Since:** PMD 5.0 - -**Priority:** Medium (3) - -Using == in condition may lead to unexpected results, as the variables are automatically casted to be of the -same type. The === operator avoids the casting. - -``` -//InfixExpression[(@Image = "==" or @Image = "!=") - and - (child::KeywordLiteral[@Image = "true" or @Image = "false"] - or - child::NumberLiteral) -] -``` - -**Example(s):** - -``` javascript -// Ok -if (someVar === true) { - ... -} -// Ok -if (someVar !== 3) { - ... -} -// Bad -if (someVar == true) { - ... -} -// Bad -if (someVar != 3) { - ... -} -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## GlobalVariable - -**Since:** PMD 5.0 - -**Priority:** High (1) - -This rule helps to avoid using accidently global variables by simply missing the "var" declaration. -Global variables can lead to side-effects that are hard to debug. - -``` -//Assignment[Name/@GlobalName = 'true'] -``` - -**Example(s):** - -``` javascript -function(arg) { - notDeclaredVariable = 1; // this will create a global variable and trigger the rule - - var someVar = 1; // this is a local variable, that's ok - - window.otherGlobal = 2; // this will not trigger the rule, although it is a global variable. -} -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## InnaccurateNumericLiteral - -**Since:** PMD 5.0 - -**Priority:** Medium High (2) - -The numeric literal will have at different value at runtime, which can happen if you provide too much -precision in a floating point number. This may result in numeric calculations being in error. - -``` -//NumberLiteral[ - @Image != @Number - and translate(@Image, "e", "E") != @Number - and concat(@Image, ".0") != @Number - and @Image != substring-before(translate(@Number, ".", ""), "E")] -``` - -**Example(s):** - -``` javascript -var a = 9; // Ok -var b = 999999999999999; // Ok -var c = 999999999999999999999; // Not good -var w = 1.12e-4; // Ok -var x = 1.12; // Ok -var y = 1.1234567890123; // Ok -var z = 1.12345678901234567; // Not good -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## ScopeForInVariable - -**Since:** PMD 5.0 - -**Priority:** High (1) - -A for-in loop in which the variable name is not explicitly scoped to the enclosing scope with the 'var' keyword can -refer to a variable in an enclosing scope outside the nearest enclosing scope. This will overwrite the -existing value of the variable in the outer scope when the body of the for-in is evaluated. When the for-in loop -has finished, the variable will contain the last value used in the for-in, and the original value from before -the for-in loop will be gone. Since the for-in variable name is most likely intended to be a temporary name, it -is better to explicitly scope the variable name to the nearest enclosing scope with 'var'. - -``` -//ForInLoop[not(child::VariableDeclaration)]/Name[1] -``` - -**Example(s):** - -``` javascript -// Ok -function foo() { - var p = 'clean'; - function() { - var obj = { dirty: 'dirty' }; - for (var p in obj) { // Use 'var' here. - obj[p] = obj[p]; - } - return x; - }(); - - // 'p' still has value of 'clean'. -} -// Bad -function bar() { - var p = 'clean'; - function() { - var obj = { dirty: 'dirty' }; - for (p in obj) { // Oh no, missing 'var' here! - obj[p] = obj[p]; - } - return x; - }(); - - // 'p' is trashed and has value of 'dirty'! -} -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## UnreachableCode - -**Since:** PMD 5.0 - -**Priority:** High (1) - -A 'return', 'break', 'continue', or 'throw' statement should be the last in a block. Statements after these -will never execute. This is a bug, or extremely poor style. - -``` -//ReturnStatement[following-sibling::node()] -| - //ContinueStatement[following-sibling::node()] -| - //BreakStatement[following-sibling::node()] -| - //ThrowStatement[following-sibling::node()] -``` - -**Example(s):** - -``` javascript -// Ok -function foo() { - return 1; -} -// Bad -function bar() { - var x = 1; - return x; - x = 2; -} -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## UseBaseWithParseInt - -**Since:** PMD 5.0.1 - -**Priority:** High (1) - -This rule checks for usages of parseInt. While the second parameter is optional and usually defaults -to 10 (base/radix is 10 for a decimal number), different implementations may behave differently. -It also improves readability, if the base is given. - -See also: [parseInt()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt) - -``` -//FunctionCall/Name[ - @Image = 'parseInt' - and - count(../*) < 3 -] -``` - -**Example(s):** - -``` javascript -parseInt("010"); // unclear, could be interpreted as 10 or 7 (with a base of 7) - -parseInt("10", 10); // good -``` - -**Use this rule by referencing it:** -``` xml - -``` - diff --git a/docs/pages/pmd/rules/ecmascript/bestpractices.md b/docs/pages/pmd/rules/ecmascript/bestpractices.md new file mode 100644 index 0000000000..d4cd1f0aaf --- /dev/null +++ b/docs/pages/pmd/rules/ecmascript/bestpractices.md @@ -0,0 +1,195 @@ +--- +title: Best Practices +summary: +permalink: pmd_rules_ecmascript_bestpractices.html +folder: pmd/rules/ecmascript +sidebaractiveurl: /pmd_rules_ecmascript.html +editmepath: ../pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml +keywords: Best Practices, AvoidWithStatement, ConsistentReturn, GlobalVariable, ScopeForInVariable, UseBaseWithParseInt +--- +## AvoidWithStatement + +**Since:** PMD 5.0.1 + +**Priority:** High (1) + +Avoid using with - it's bad news + +``` +//WithStatement +``` + +**Example(s):** + +``` javascript +with (object) { + property = 3; // Might be on object, might be on window: who knows. +} +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## ConsistentReturn + +**Since:** PMD 5.0 + +**Priority:** Medium High (2) + +ECMAScript does provide for return types on functions, and therefore there is no solid rule as to their usage. +However, when a function does use returns they should all have a value, or all with no value. Mixed return +usage is likely a bug, or at best poor style. + +**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.ecmascript.rule.bestpractices.ConsistentReturnRule](https://github.com/pmd/pmd/blob/master/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/rule/bestpractices/ConsistentReturnRule.java) + +**Example(s):** + +``` javascript +// Ok +function foo() { + if (condition1) { + return true; + } + return false; +} + +// Bad +function bar() { + if (condition1) { + return; + } + return false; +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description| +|----|-------------|-----------| +|rhinoLanguageVersion|VERSION_DEFAULT|Specifies the Rhino Language Version to use for parsing. Defaults to Rhino default.| +|recordingLocalJsDocComments|true|Specifies that JsDoc comments are produced in the AST.| +|recordingComments|true|Specifies that comments are produced in the AST.| + +**Use this rule by referencing it:** +``` xml + +``` + +## GlobalVariable + +**Since:** PMD 5.0 + +**Priority:** High (1) + +This rule helps to avoid using accidently global variables by simply missing the "var" declaration. +Global variables can lead to side-effects that are hard to debug. + +``` +//Assignment[Name/@GlobalName = 'true'] +``` + +**Example(s):** + +``` javascript +function(arg) { + notDeclaredVariable = 1; // this will create a global variable and trigger the rule + + var someVar = 1; // this is a local variable, that's ok + + window.otherGlobal = 2; // this will not trigger the rule, although it is a global variable. +} +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## ScopeForInVariable + +**Since:** PMD 5.0 + +**Priority:** High (1) + +A for-in loop in which the variable name is not explicitly scoped to the enclosing scope with the 'var' keyword can +refer to a variable in an enclosing scope outside the nearest enclosing scope. This will overwrite the +existing value of the variable in the outer scope when the body of the for-in is evaluated. When the for-in loop +has finished, the variable will contain the last value used in the for-in, and the original value from before +the for-in loop will be gone. Since the for-in variable name is most likely intended to be a temporary name, it +is better to explicitly scope the variable name to the nearest enclosing scope with 'var'. + +``` +//ForInLoop[not(child::VariableDeclaration)]/Name[1] +``` + +**Example(s):** + +``` javascript +// Ok +function foo() { + var p = 'clean'; + function() { + var obj = { dirty: 'dirty' }; + for (var p in obj) { // Use 'var' here. + obj[p] = obj[p]; + } + return x; + }(); + + // 'p' still has value of 'clean'. +} +// Bad +function bar() { + var p = 'clean'; + function() { + var obj = { dirty: 'dirty' }; + for (p in obj) { // Oh no, missing 'var' here! + obj[p] = obj[p]; + } + return x; + }(); + + // 'p' is trashed and has value of 'dirty'! +} +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## UseBaseWithParseInt + +**Since:** PMD 5.0.1 + +**Priority:** High (1) + +This rule checks for usages of parseInt. While the second parameter is optional and usually defaults +to 10 (base/radix is 10 for a decimal number), different implementations may behave differently. +It also improves readability, if the base is given. + +See also: [parseInt()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt) + +``` +//FunctionCall/Name[ + @Image = 'parseInt' + and + count(../*) < 3 +] +``` + +**Example(s):** + +``` javascript +parseInt("010"); // unclear, could be interpreted as 10 or 7 (with a base of 7) + +parseInt("10", 10); // good +``` + +**Use this rule by referencing it:** +``` xml + +``` + diff --git a/docs/pages/pmd/rules/ecmascript/braces.md b/docs/pages/pmd/rules/ecmascript/braces.md deleted file mode 100644 index 5f67d6dcf7..0000000000 --- a/docs/pages/pmd/rules/ecmascript/braces.md +++ /dev/null @@ -1,137 +0,0 @@ ---- -title: Braces -summary: The Braces Ruleset contains a collection of braces rules. -permalink: pmd_rules_ecmascript_braces.html -folder: pmd/rules/ecmascript -sidebaractiveurl: /pmd_rules_ecmascript.html -editmepath: ../pmd-javascript/src/main/resources/rulesets/ecmascript/braces.xml -keywords: Braces, IfStmtsMustUseBraces, IfElseStmtsMustUseBraces, WhileLoopsMustUseBraces, ForLoopsMustUseBraces ---- -## ForLoopsMustUseBraces - -**Since:** PMD 5.0 - -**Priority:** Medium (3) - -Avoid using 'for' statements without using curly braces. - -``` -//ForLoop[not(child::Scope)] -| -//ForInLoop[not(child::Scope)] -``` - -**Example(s):** - -``` javascript -// Ok -for (var i = 0; i < 42; i++) { - foo(); -} - -// Bad -for (var i = 0; i < 42; i++) - foo(); -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## IfElseStmtsMustUseBraces - -**Since:** PMD 5.0 - -**Priority:** Medium (3) - -Avoid using if..else statements without using curly braces. - -``` -//ExpressionStatement[parent::IfStatement[@Else = "true"]] - [not(child::Scope)] - [not(child::IfStatement)] -``` - -**Example(s):** - -``` javascript -// Ok -if (foo) { - x++; -} else { - y++; -} - -// Bad -if (foo) - x++; -else - y++; -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## IfStmtsMustUseBraces - -**Since:** PMD 5.0 - -**Priority:** Medium (3) - -Avoid using if statements without using curly braces. - -``` -//IfStatement[@Else = "false" and not(child::Scope)] -``` - -**Example(s):** - -``` javascript -// Ok -if (foo) { - x++; -} - -// Bad -if (foo) - x++; -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## WhileLoopsMustUseBraces - -**Since:** PMD 5.0 - -**Priority:** Medium (3) - -Avoid using 'while' statements without using curly braces. - -``` -//WhileLoop[not(child::Scope)] -``` - -**Example(s):** - -``` javascript -// Ok -while (true) { - x++; -} - -// Bad -while (true) - x++; -``` - -**Use this rule by referencing it:** -``` xml - -``` - diff --git a/docs/pages/pmd/rules/ecmascript/codestyle.md b/docs/pages/pmd/rules/ecmascript/codestyle.md new file mode 100644 index 0000000000..9d0fcfb44f --- /dev/null +++ b/docs/pages/pmd/rules/ecmascript/codestyle.md @@ -0,0 +1,365 @@ +--- +title: Code Style +summary: +permalink: pmd_rules_ecmascript_codestyle.html +folder: pmd/rules/ecmascript +sidebaractiveurl: /pmd_rules_ecmascript.html +editmepath: ../pmd-javascript/src/main/resources/category/ecmascript/codestyle.xml +keywords: Code Style, AssignmentInOperand, AvoidTrailingComma, ForLoopsMustUseBraces, IfElseStmtsMustUseBraces, IfStmtsMustUseBraces, NoElseReturn, UnnecessaryBlock, UnnecessaryParentheses, UnreachableCode, WhileLoopsMustUseBraces +--- +## AssignmentInOperand + +**Since:** PMD 5.0 + +**Priority:** Medium High (2) + +Avoid assignments in operands; this can make code more complicated and harder to read. This is sometime +indicative of the bug where the assignment operator '=' was used instead of the equality operator '=='. + +``` +//IfStatement[$allowIf = "false"]/child::node()[1]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] +| + //WhileLoop[$allowWhile = "false"]/child::node()[1]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] +| + //DoLoop[$allowWhile = "false"]/child::node()[2]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] +| + //ForLoop[$allowFor = "false"]/child::node()[2]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] +| + //ConditionalExpression[$allowTernary = "false"]/child::node()[1]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] +| + //ConditionalExpression[$allowTernaryResults = "false"]/child::node()[position() = 2 or position() = 3]/descendant-or-self::node()[self::Assignment or self::UnaryExpression[$allowIncrementDecrement = "false" and (@Image = "--" or @Image = "++")]] +``` + +**Example(s):** + +``` javascript +var x = 2; +// Bad +if ((x = getX()) == 3) { + alert('3!'); +} + +function getX() { + return 3; +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description| +|----|-------------|-----------| +|allowIf|false|Allow assignment within the conditional expression of an if statement| +|allowFor|false|Allow assignment within the conditional expression of a for statement| +|allowWhile|false|Allow assignment within the conditional expression of a while statement| +|allowTernary|false|Allow assignment within the conditional expression of a ternary operator| +|allowTernaryResults|false|Allow assignment within the result expressions of a ternary operator| +|allowIncrementDecrement|false|Allow increment or decrement operators within the conditional expression of an if, for, or while statement| + +**Use this rule by referencing it:** +``` xml + +``` + +## AvoidTrailingComma + +**Since:** PMD 5.1 + +**Priority:** High (1) + +This rule helps improve code portability due to differences in browser treatment of trailing commas in object or array literals. + +``` +//ObjectLiteral[$allowObjectLiteral = "false" and @TrailingComma = 'true'] +| +//ArrayLiteral[$allowArrayLiteral = "false" and @TrailingComma = 'true'] +``` + +**Example(s):** + +``` javascript +function(arg) { + var obj1 = { a : 1 }; // Ok + var arr1 = [ 1, 2 ]; // Ok + + var obj2 = { a : 1, }; // Syntax error in some browsers! + var arr2 = [ 1, 2, ]; // Length 2 or 3 depending on the browser! +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description| +|----|-------------|-----------| +|allowObjectLiteral|false|Allow a trailing comma within an object literal| +|allowArrayLiteral|false|Allow a trailing comma within an array literal| + +**Use this rule by referencing it:** +``` xml + +``` + +## ForLoopsMustUseBraces + +**Since:** PMD 5.0 + +**Priority:** Medium (3) + +Avoid using 'for' statements without using curly braces. + +``` +//ForLoop[not(child::Scope)] +| +//ForInLoop[not(child::Scope)] +``` + +**Example(s):** + +``` javascript +// Ok +for (var i = 0; i < 42; i++) { + foo(); +} + +// Bad +for (var i = 0; i < 42; i++) + foo(); +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## IfElseStmtsMustUseBraces + +**Since:** PMD 5.0 + +**Priority:** Medium (3) + +Avoid using if..else statements without using curly braces. + +``` +//ExpressionStatement[parent::IfStatement[@Else = "true"]] + [not(child::Scope)] + [not(child::IfStatement)] +``` + +**Example(s):** + +``` javascript +// Ok +if (foo) { + x++; +} else { + y++; +} + +// Bad +if (foo) + x++; +else + y++; +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## IfStmtsMustUseBraces + +**Since:** PMD 5.0 + +**Priority:** Medium (3) + +Avoid using if statements without using curly braces. + +``` +//IfStatement[@Else = "false" and not(child::Scope)] +``` + +**Example(s):** + +``` javascript +// Ok +if (foo) { + x++; +} + +// Bad +if (foo) + x++; +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## NoElseReturn + +**Since:** PMD 5.5.0 + +**Priority:** Medium (3) + +The else block in a if-else-construct is unnecessary if the `if` block contains a return. +Then the content of the else block can be put outside. + +See also: + +``` +//IfStatement[@Else="true"][Scope[1]/ReturnStatement] +``` + +**Example(s):** + +``` javascript +// Bad: +if (x) { + return y; +} else { + return z; +} + +// Good: +if (x) { + return y; +} +return z; +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## UnnecessaryBlock + +**Since:** PMD 5.0 + +**Priority:** Medium (3) + +An unnecessary Block is present. Such Blocks are often used in other languages to +introduce a new variable scope. Blocks do not behave like this in ECMAScipt, and using them can +be misleading. Considering removing this unnecessary Block. + +``` +//Block[not(parent::FunctionNode or parent::IfStatement or parent::ForLoop or parent::ForInLoop + or parent::WhileLoop or parent::DoLoop or parent::TryStatement or parent::CatchClause)] +| +//Scope[not(parent::FunctionNode or parent::IfStatement or parent::ForLoop or parent::ForInLoop + or parent::WhileLoop or parent::DoLoop or parent::TryStatement or parent::CatchClause)] +``` + +**Example(s):** + +``` javascript +if (foo) { + // Ok +} +if (bar) { + { + // Bad + } +} +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## UnnecessaryParentheses + +**Since:** PMD 5.0 + +**Priority:** Medium Low (4) + +Unnecessary parentheses should be removed. + +``` +//ParenthesizedExpression/ParenthesizedExpression +``` + +**Example(s):** + +``` javascript +var x = 1; // Ok +var y = (1 + 1); // Ok +var z = ((1 + 1)); // Bad +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## UnreachableCode + +**Since:** PMD 5.0 + +**Priority:** High (1) + +A 'return', 'break', 'continue', or 'throw' statement should be the last in a block. Statements after these +will never execute. This is a bug, or extremely poor style. + +``` +//ReturnStatement[following-sibling::node()] +| + //ContinueStatement[following-sibling::node()] +| + //BreakStatement[following-sibling::node()] +| + //ThrowStatement[following-sibling::node()] +``` + +**Example(s):** + +``` javascript +// Ok +function foo() { + return 1; +} +// Bad +function bar() { + var x = 1; + return x; + x = 2; +} +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## WhileLoopsMustUseBraces + +**Since:** PMD 5.0 + +**Priority:** Medium (3) + +Avoid using 'while' statements without using curly braces. + +``` +//WhileLoop[not(child::Scope)] +``` + +**Example(s):** + +``` javascript +// Ok +while (true) { + x++; +} + +// Bad +while (true) + x++; +``` + +**Use this rule by referencing it:** +``` xml + +``` + diff --git a/docs/pages/pmd/rules/ecmascript/controversial.md b/docs/pages/pmd/rules/ecmascript/controversial.md deleted file mode 100644 index b73ea9d35b..0000000000 --- a/docs/pages/pmd/rules/ecmascript/controversial.md +++ /dev/null @@ -1,34 +0,0 @@ ---- -title: Controversial Ecmascript -summary: The Controversial ruleset contains rules that, for whatever reason, are considered controversial. They are held here to allow people to include them as they see fit within their custom rulesets. -permalink: pmd_rules_ecmascript_controversial.html -folder: pmd/rules/ecmascript -sidebaractiveurl: /pmd_rules_ecmascript.html -editmepath: ../pmd-javascript/src/main/resources/rulesets/ecmascript/controversial.xml -keywords: Controversial Ecmascript, AvoidWithStatement ---- -## AvoidWithStatement - -**Since:** PMD 5.0.1 - -**Priority:** High (1) - -Avoid using with - it's bad news - -``` -//WithStatement -``` - -**Example(s):** - -``` javascript -with (object) { - property = 3; // Might be on object, might be on window: who knows. -} -``` - -**Use this rule by referencing it:** -``` xml - -``` - diff --git a/docs/pages/pmd/rules/ecmascript/errorprone.md b/docs/pages/pmd/rules/ecmascript/errorprone.md new file mode 100644 index 0000000000..6c5a48604d --- /dev/null +++ b/docs/pages/pmd/rules/ecmascript/errorprone.md @@ -0,0 +1,87 @@ +--- +title: Error Prone +summary: +permalink: pmd_rules_ecmascript_errorprone.html +folder: pmd/rules/ecmascript +sidebaractiveurl: /pmd_rules_ecmascript.html +editmepath: ../pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml +keywords: Error Prone, EqualComparison, InnaccurateNumericLiteral +--- +## EqualComparison + +**Since:** PMD 5.0 + +**Priority:** Medium (3) + +Using == in condition may lead to unexpected results, as the variables are automatically casted to be of the +same type. The === operator avoids the casting. + +``` +//InfixExpression[(@Image = "==" or @Image = "!=") + and + (child::KeywordLiteral[@Image = "true" or @Image = "false"] + or + child::NumberLiteral) +] +``` + +**Example(s):** + +``` javascript +// Ok +if (someVar === true) { + ... +} +// Ok +if (someVar !== 3) { + ... +} +// Bad +if (someVar == true) { + ... +} +// Bad +if (someVar != 3) { + ... +} +``` + +**Use this rule by referencing it:** +``` xml + +``` + +## InnaccurateNumericLiteral + +**Since:** PMD 5.0 + +**Priority:** Medium High (2) + +The numeric literal will have a different value at runtime, which can happen if you provide too much +precision in a floating point number. This may result in numeric calculations being in error. + +``` +//NumberLiteral[ + @Image != @Number + and translate(@Image, "e", "E") != @Number + and concat(@Image, ".0") != @Number + and @Image != substring-before(translate(@Number, ".", ""), "E")] +``` + +**Example(s):** + +``` javascript +var a = 9; // Ok +var b = 999999999999999; // Ok +var c = 999999999999999999999; // Not good +var w = 1.12e-4; // Ok +var x = 1.12; // Ok +var y = 1.1234567890123; // Ok +var z = 1.12345678901234567; // Not good +``` + +**Use this rule by referencing it:** +``` xml + +``` + diff --git a/docs/pages/pmd/rules/ecmascript/unnecessary.md b/docs/pages/pmd/rules/ecmascript/unnecessary.md deleted file mode 100644 index 6c5c0f5ec0..0000000000 --- a/docs/pages/pmd/rules/ecmascript/unnecessary.md +++ /dev/null @@ -1,107 +0,0 @@ ---- -title: Unnecessary -summary: The Unnecessary Ruleset contains a collection of rules for unnecessary code. -permalink: pmd_rules_ecmascript_unnecessary.html -folder: pmd/rules/ecmascript -sidebaractiveurl: /pmd_rules_ecmascript.html -editmepath: ../pmd-javascript/src/main/resources/rulesets/ecmascript/unnecessary.xml -keywords: Unnecessary, UnnecessaryParentheses, UnnecessaryBlock, NoElseReturn ---- -## NoElseReturn - -**Since:** PMD 5.5.0 - -**Priority:** Medium (3) - -The else block in a if-else-construct is unnecessary if the `if` block contains a return. -Then the content of the else block can be put outside. - -See also: - -``` -//IfStatement[@Else="true"][Scope[1]/ReturnStatement] -``` - -**Example(s):** - -``` javascript -// Bad: -if (x) { - return y; -} else { - return z; -} - -// Good: -if (x) { - return y; -} -return z; -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## UnnecessaryBlock - -**Since:** PMD 5.0 - -**Priority:** Medium (3) - -An unnecessary Block is present. Such Blocks are often used in other languages to -introduce a new variable scope. Blocks do not behave like this in ECMAScipt, and using them can -be misleading. Considering removing this unnecessary Block. - -``` -//Block[not(parent::FunctionNode or parent::IfStatement or parent::ForLoop or parent::ForInLoop - or parent::WhileLoop or parent::DoLoop or parent::TryStatement or parent::CatchClause)] -| -//Scope[not(parent::FunctionNode or parent::IfStatement or parent::ForLoop or parent::ForInLoop - or parent::WhileLoop or parent::DoLoop or parent::TryStatement or parent::CatchClause)] -``` - -**Example(s):** - -``` javascript -if (foo) { - // Ok -} -if (bar) { - { - // Bad - } -} -``` - -**Use this rule by referencing it:** -``` xml - -``` - -## UnnecessaryParentheses - -**Since:** PMD 5.0 - -**Priority:** Medium Low (4) - -Unnecessary parentheses should be removed. - -``` -//ParenthesizedExpression/ParenthesizedExpression -``` - -**Example(s):** - -``` javascript -var x = 1; // Ok -var y = (1 + 1); // Ok -var z = ((1 + 1)); // Bad -``` - -**Use this rule by referencing it:** -``` xml - -``` -