hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Merge branch 'pr-1236'

Browse Source
This commit is contained in:
Juan Martín Sotuyo Dodero
2018-07-30 02:01:49 -03:00
parent ecdd3ad2a7 369c3fa0a0
commit 3820bdc36c
15 changed files with 486 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
docs/pages/release_notes.md
View File

@ -13,16 +13,25 @@ This is a minor release.
### Table Of Contents
* [New and noteworthy](#new-and-noteworthy)
* [New Rules](#new-rules)
* [Fixed Issues](#fixed-issues)
* [API Changes](#api-changes)
* [External Contributions](#external-contributions)
### New and noteworthy
#### New Rules
* The new PL/SQL rule [`ForLoopNaming`](pmd_rules_plsql_codestyle.html#forloopnaming) (`plsql-codestyle`)
enforces a naming convention for "for loops". Both "cursor for loops" and "index for loops" are covered.
The rule can be customized via patterns. By default, short variable names are reported.
### Fixed Issues
* java-errorprone
* [#1078](https://github.com/pmd/pmd/issues/1078): \[java] MissingSerialVersionUID rule does not seem to catch inherited classes
* plsql
* [#681](https://github.com/pmd/pmd/issues/681): \[plsql] Parse error with Cursor For Loop
### API Changes

13
pmd-core/src/main/resources/rulesets/releases/670.xml Normal file
View File

@ -0,0 +1,13 @@
<?xml version="1.0"?>
<ruleset name="670"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
This ruleset contains links to rules that are new in PMD v6.7.0
</description>
<rule ref="category/plsql/codestyle.xml/ForLoopNaming"/>
</ruleset>

20
pmd-plsql/etc/grammar/PldocAST.jjt
View File

@ -28,6 +28,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
/**
* Added more complete support for CREATE TABLE
* Added ASTCursorForLoop and ASTSelectStatement
*
* Andreas Dangel 07/2018
*====================================================================
@ -1176,6 +1177,7 @@ ASTUnlabelledStatement UnlabelledStatement() :
| LOOKAHEAD(3) ContinueStatement() ";" // CONTINUE keyword was added in 11G, so Oracle compilation supports CONTINUE as a variable name
| CaseStatement() ";"
| IfStatement() ";"
| LOOKAHEAD(<FOR> ID() <IN> "(" <SELECT>) CursorForLoopStatement() ";"
| ForStatement() ";"
| ForAllStatement() ";"
| LoopStatement() ";"
@ -1273,6 +1275,20 @@ ASTLoopStatement LoopStatement() :
{ return jjtThis ; }
}
ASTCursorForLoopStatement CursorForLoopStatement() :
{}
{
<FOR> ForIndex() <IN> "(" SelectStatement() ")" <LOOP> (Statement())+ <END> <LOOP> [<IDENTIFIER>]
{ return jjtThis ; }
}
ASTSelectStatement SelectStatement() :
{}
{
SqlStatement("(", ")")
{ return jjtThis ; }
}
/** Scope rule: the loop index only exists within the Loop */
ASTForStatement ForStatement() :
{}
@ -3732,7 +3748,7 @@ TOKEN [IGNORE_CASE]:
*/
TOKEN :
{
< #GERMAN_SPECIAL_CHARACTERS: "Ä" | "Ö" | "Ü" | "ä" | "ü" | "ö" | "ß" >
< #GERMAN_SPECIAL_CHARACTERS: "Ä" | "Ö" | "Ü" | "ä" | "ü" | "ö" | "ß" >
|
< #LETTER: ["A"-"Z"] | ["a"-"z"] | <GERMAN_SPECIAL_CHARACTERS> >
|
@ -3745,7 +3761,7 @@ TOKEN :
| "$" | "&" | "_" | "|" | "{" | "}" | "?" | "[" | "]"
| " " | "\t" >
|
< #SPECIAL_CHARACTERS: "á" | "Ž" | "™" | "š" | "„" | "”" | "ý" | "²" | "€" | "³" | "µ">
< #SPECIAL_CHARACTERS: "á" | "Ž" | "™" | "š" | "„" | "”" | "ý" | "²" | "€" | "³" | "µ">
|
< #DELIMITER: "+" | "%" | "'" | "\"" | "." | "/" | "(" | ")" | ":" | "," | "*" | "=" | "<" | ">" | "@" | ";" | "-">
|

10
pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/ast/PLSQLParserVisitorAdapter.java
View File

@ -710,4 +710,14 @@ public class PLSQLParserVisitorAdapter implements PLSQLParserVisitor {
public Object visit(ASTReferencesClause node, Object data) {
return visit((PLSQLNode) node, data);
}
@Override
public Object visit(ASTCursorForLoopStatement node, Object data) {
return visit((PLSQLNode) node, data);
}
@Override
public Object visit(ASTSelectStatement node, Object data) {
return visit((PLSQLNode) node, data);
}
}

10
pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/rule/AbstractPLSQLRule.java
View File

@ -805,6 +805,16 @@ public abstract class AbstractPLSQLRule extends AbstractRule implements PLSQLPar
return visit((PLSQLNode) node, data);
}
@Override
public Object visit(ASTCursorForLoopStatement node, Object data) {
return visit((PLSQLNode) node, data);
}
@Override
public Object visit(ASTSelectStatement node, Object data) {
return visit((PLSQLNode) node, data);
}
/*
* Treat all Executable Code
*/

60
pmd-plsql/src/main/resources/category/plsql/codestyle.xml
View File

@ -52,4 +52,64 @@ end inline_pragma_error;
</example>
</rule>
<rule name="ForLoopNaming"
language="plsql"
since="6.7.0"
message="Use meaningful names for loop variables"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_plsql_codestyle.html#forloopnaming">
<description>
In case you have loops please name the loop variables more meaningful.
</description>
<priority>3</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
//CursorForLoopStatement[
$allowSimpleLoops = 'false' or
(Statement//CursorForLoopStatement or ancestor::CursorForLoopStatement)
]
/ForIndex[not(matches(@Image, $cursorPattern))]
|
//ForStatement[
$allowSimpleLoops = 'false' or
(Statement//ForStatement or ancestor::ForStatement)
]
/ForIndex[not(matches(@Image, $indexPattern))]
]]>
</value>
</property>
<property name="allowSimpleLoops" type="Boolean" description="Ignore simple loops, that are not nested" value="false" />
<property name="cursorPattern" type="Regex" description="The pattern used for the curosr loop variable" value="[a-zA-Z_0-9]{5,}" />
<property name="indexPattern" type="Regex" description="The pattern used for the index loop variable" value="[a-zA-Z_0-9]{5,}" />
</properties>
<example>
<![CDATA[
-- good example
BEGIN
FOR company IN (SELECT * FROM companies) LOOP
FOR contact IN (SELECT * FROM contacts) LOOP
FOR party IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
-- bad example
BEGIN
FOR c1 IN (SELECT * FROM companies) LOOP
FOR c2 IN (SELECT * FROM contacts) LOOP
FOR c3 IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]>
</example>
</rule>
</ruleset>

11
pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/AbstractPLSQLParserTst.java
View File

@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.plsql;
import java.io.IOException;
import java.io.StringReader;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
@ -15,6 +16,8 @@ import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.apache.commons.io.IOUtils;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.lang.LanguageVersionHandler;
@ -127,4 +130,12 @@ public abstract class AbstractPLSQLParserTst {
return languageVersionHandler.getParser(languageVersionHandler.getDefaultParserOptions()).parse(null,
new StringReader(code));
}
public String loadTestResource(String name) {
try {
return IOUtils.toString(this.getClass().getResourceAsStream(name));
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}

64
pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopTest.java Normal file
View File

@ -0,0 +1,64 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.plsql.ast;
import org.junit.Assert;
import org.junit.Test;
import net.sourceforge.pmd.lang.plsql.AbstractPLSQLParserTst;
public class CursorForLoopTest extends AbstractPLSQLParserTst {
@Test
public void parseCursorForLoopSimple() {
String code = loadTestResource("CursorForLoopSimple.pls");
ASTInput input = parsePLSQL(code);
ASTCursorForLoopStatement forloop = input.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
Assert.assertNotNull(forloop);
ASTForIndex forindex = forloop.getFirstChildOfType(ASTForIndex.class);
Assert.assertNotNull(forindex);
Assert.assertEquals("someone", forindex.getImage());
}
@Test
public void parseCursorForLoopNested() {
String code = loadTestResource("CursorForLoopNested.pls");
ASTInput input = parsePLSQL(code);
ASTCursorForLoopStatement forloop = input.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
Assert.assertNotNull(forloop);
ASTForIndex forindex = forloop.getFirstChildOfType(ASTForIndex.class);
Assert.assertNotNull(forindex);
Assert.assertEquals("c_cmp", forindex.getImage());
ASTCursorForLoopStatement forloop2 = forloop.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
ASTForIndex forindex2 = forloop2.getFirstChildOfType(ASTForIndex.class);
Assert.assertEquals("c_con", forindex2.getImage());
ASTCursorForLoopStatement forloop3 = forloop2.getFirstDescendantOfType(ASTCursorForLoopStatement.class);
ASTForIndex forindex3 = forloop3.getFirstChildOfType(ASTForIndex.class);
Assert.assertEquals("c_pa", forindex3.getImage());
}
@Test
public void parseCursorForLoop1047a() {
String code = loadTestResource("CursorForLoop1047a.pls");
ASTInput input = parsePLSQL(code);
Assert.assertNotNull(input);
}
@Test
public void parseCursorForLoop1047b() {
String code = loadTestResource("CursorForLoop1047b.pls");
ASTInput input = parsePLSQL(code);
Assert.assertNotNull(input);
}
@Test
public void parseCursorForLoop681() {
String code = loadTestResource("CursorForLoop681.pls");
ASTInput input = parsePLSQL(code);
Assert.assertNotNull(input);
}
}

1
pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/rule/codestyle/CodeStyleRulesTest.java
View File

@ -13,5 +13,6 @@ public class CodeStyleRulesTest extends SimpleAggregatorTst {
@Override
public void setUp() {
addRule(RULESET, "MisplacedPragma");
addRule(RULESET, "ForLoopNaming");
}
}

16
pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047a.pls Normal file
View File

@ -0,0 +1,16 @@
--
-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
--
CREATE OR REPLACE PROCEDURE test2 ( a OUT number, b OUT number )
AS
BEGIN
FOR registro IN ( SELECT SUM(field1),
MAX(field2)
INTO a,b
FROM test_tbl
GROUP BY fieldx)
LOOP
null;
END LOOP;
END test2;

13
pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop1047b.pls Normal file
View File

@ -0,0 +1,13 @@
--
-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
--
-- this one was processed without errors
CREATE OR REPLACE PROCEDURE test2 ( a OUT number, b OUT number )
AS
BEGIN
FOR registro IN ( SELECT SUM(field1),MAX(field2) INTO a,b FROM test_tbl GROUP BY fieldx )
LOOP
null;
END LOOP;
END test2;

20
pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoop681.pls Normal file
View File

@ -0,0 +1,20 @@
--
-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
--
BEGIN
FOR sum_res IN (SELECT SUM(loss_reserve) loss_reserve,
SUM(expense_reserve) exp_reserve
FROM gicl_clm_reserve a, gicl_item_peril b
WHERE a.claim_id = b.claim_id
AND a.item_no = b.item_no
AND a.peril_cd = b.peril_cd
AND a.claim_id = p_claim_id
AND (NVL(b.close_flag, 'AP') IN ('AP','CC','CP') OR
NVL(b.close_flag2, 'AP') IN ('AP','CC','CP')))
LOOP
v_loss_res_amt := sum_res.loss_reserve;
v_exp_res_amt := sum_res.exp_reserve;
END LOOP;
END;
/

14
pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopNested.pls Normal file
View File

@ -0,0 +1,14 @@
--
-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
--
BEGIN
FOR c_cmp IN (SELECT * FROM companies) LOOP
FOR c_con IN (SELECT * FROM contacts) LOOP
FOR c_pa IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/

16
pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/CursorForLoopSimple.pls Normal file
View File

@ -0,0 +1,16 @@
--
-- BSD-style license; for more info see http://pmd.sourceforge.net/license.html
--
BEGIN
FOR someone IN (
SELECT * FROM employees
WHERE employee_id < 120
ORDER BY employee_id
)
LOOP
DBMS_OUTPUT.PUT_LINE('First name = ' || someone.first_name ||
', Last name = ' || someone.last_name);
END LOOP;
END;
/

211
pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/rule/codestyle/xml/ForLoopNaming.xml Normal file
View File

@ -0,0 +1,211 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description>CursorForLoop Simple case - no nesting</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
BEGIN
FOR c IN (SELECT * FROM companies) LOOP
NULL;
END LOOP;
END;
/
]]></code>
<source-type>plsql</source-type>
</test-code>
<test-code>
<description>CursorForLoop Simple case - no nesting - allowSimpleLoops</description>
<rule-property name="allowSimpleLoops">true</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
BEGIN
FOR c IN (SELECT * FROM companies) LOOP
NULL;
END LOOP;
END;
/
]]></code>
<source-type>plsql</source-type>
</test-code>
<test-code>
<description>CursorForLoop Nested loops - good example, default pattern</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
BEGIN
FOR company IN (SELECT * FROM companies) LOOP
FOR contact IN (SELECT * FROM contacts) LOOP
FOR party IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
<source-type>plsql</source-type>
</test-code>
<test-code>
<description>CursorForLoop Nested loops - good example, custom pattern</description>
<rule-property name="cursorPattern">c_[a-z]+</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
BEGIN
FOR c_cmp IN (SELECT * FROM companies) LOOP
FOR c_con IN (SELECT * FROM contacts) LOOP
FOR c_pa IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
<source-type>plsql</source-type>
</test-code>
<test-code>
<description>CursorForLoop Nested loops - bad example, default pattern</description>
<expected-problems>3</expected-problems>
<expected-linenumbers>2,3,4</expected-linenumbers>
<code><![CDATA[
BEGIN
FOR cmp IN (SELECT * FROM companies) LOOP
FOR con IN (SELECT * FROM contacts) LOOP
FOR pa IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
<source-type>plsql</source-type>
</test-code>
<test-code>
<description>CursorForLoop Nested loops - bad example, custom pattern</description>
<rule-property name="cursorPattern">c_[a-z]+</rule-property>
<expected-problems>3</expected-problems>
<expected-linenumbers>2,3,4</expected-linenumbers>
<code><![CDATA[
BEGIN
FOR c1 IN (SELECT * FROM companies) LOOP
FOR c2 IN (SELECT * FROM contacts) LOOP
FOR c3 IN (SELECT * FROM parties) LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
<source-type>plsql</source-type>
</test-code>
<test-code>
<description>ForLoop Simple case - no nesting</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>2</expected-linenumbers>
<code><![CDATA[
BEGIN
FOR i IN 1..v_companies.COUNT LOOP
NULL;
END LOOP;
END;
/
]]></code>
</test-code>
<test-code>
<description>ForLoop Simple case - no nesting - allowSimpleLoops</description>
<rule-property name="allowSimpleLoops">true</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
BEGIN
FOR i IN 1..v_companies.COUNT LOOP
NULL;
END LOOP;
END;
/
]]></code>
</test-code>
<test-code>
<description>ForLoop Nested loops - good example, default pattern</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
BEGIN
FOR company IN 1..v_companies.COUNT LOOP
FOR contact IN 1..v_contacts.COUNT LOOP
FOR party IN 1..v_parties.COUNT LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
</test-code>
<test-code>
<description>ForLoop Nested loops - good example, custom pattern</description>
<rule-property name="indexPattern">i_[a-z]+</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
BEGIN
FOR i_cmp IN 1..v_companies.COUNT LOOP
FOR i_con IN 1..v_contacts.COUNT LOOP
FOR i_pa IN 1..v_parties.COUNT LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
</test-code>
<test-code>
<description>ForLoop Nested Loops - bad example, default pattern</description>
<expected-problems>3</expected-problems>
<expected-linenumbers>2,3,4</expected-linenumbers>
<code><![CDATA[
BEGIN
FOR i IN 1..v_companies.COUNT LOOP
FOR j IN 1..v_contacts.COUNT LOOP
FOR k IN 1..v_parties.COUNT LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
</test-code>
<test-code>
<description>ForLoop Nested Loops - bad example, custom pattern</description>
<rule-property name="indexPattern">i_[a-z]+</rule-property>
<expected-problems>3</expected-problems>
<expected-linenumbers>2,3,4</expected-linenumbers>
<code><![CDATA[
BEGIN
FOR i IN 1..v_companies.COUNT LOOP
FOR j IN 1..v_contacts.COUNT LOOP
FOR k IN 1..v_parties.COUNT LOOP
NULL;
END LOOP;
END LOOP;
END LOOP;
END;
/
]]></code>
</test-code>
</test-data>