Merge branch 'pr-1381'

This commit is contained in:
Andreas Dangel
2018-10-28 14:53:44 +01:00
6 changed files with 897 additions and 0 deletions

View File

@ -21,6 +21,12 @@ Thanks to the work of [ITBA](https://www.itba.edu.ar/) students [Matías Fraga](
Golang is now backed by a proper Antlr Grammar. This means CPD is now better at detecting duplicates,
as comments are recognized as such and ignored.
#### New Rules
* The new PLSQL rule {% rule "plsql/codestyle/CodeFormat" %} (`plsql-codestyle`) verifies that
PLSQL code is properly formatted. It checks e.g. for correct indentation in select statements and verifies
that each parameter is defined on a separate line.
### Fixed Issues
* all

View File

@ -0,0 +1,14 @@
<?xml version="1.0"?>
<ruleset name="690"
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 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>
This ruleset contains links to rules that are new in PMD v6.9.0
</description>
<rule ref="category/plsql/codestyle.xml/CodeFormat"/>
</ruleset>

View File

@ -0,0 +1,261 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.plsql.rule.codestyle;
import java.util.List;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.plsql.ast.ASTArgument;
import net.sourceforge.pmd.lang.plsql.ast.ASTArgumentList;
import net.sourceforge.pmd.lang.plsql.ast.ASTBulkCollectIntoClause;
import net.sourceforge.pmd.lang.plsql.ast.ASTDatatype;
import net.sourceforge.pmd.lang.plsql.ast.ASTDeclarativeSection;
import net.sourceforge.pmd.lang.plsql.ast.ASTEqualityExpression;
import net.sourceforge.pmd.lang.plsql.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.plsql.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.plsql.ast.ASTFromClause;
import net.sourceforge.pmd.lang.plsql.ast.ASTInput;
import net.sourceforge.pmd.lang.plsql.ast.ASTJoinClause;
import net.sourceforge.pmd.lang.plsql.ast.ASTSelectList;
import net.sourceforge.pmd.lang.plsql.ast.ASTSubqueryOperation;
import net.sourceforge.pmd.lang.plsql.ast.ASTUnqualifiedID;
import net.sourceforge.pmd.lang.plsql.ast.ASTVariableOrConstantDeclarator;
import net.sourceforge.pmd.lang.plsql.rule.AbstractPLSQLRule;
import net.sourceforge.pmd.properties.IntegerProperty;
public class CodeFormatRule extends AbstractPLSQLRule {
private static final IntegerProperty INDENTATION_PROPERTY = IntegerProperty.named("indentation")
.desc("Indentation to be used for blocks").defaultValue(2).range(0, 20).build();
private int indentation = INDENTATION_PROPERTY.defaultValue();
public CodeFormatRule() {
definePropertyDescriptor(INDENTATION_PROPERTY);
}
@Override
public Object visit(ASTInput node, Object data) {
indentation = getProperty(INDENTATION_PROPERTY);
return super.visit(node, data);
}
@Override
public Object visit(ASTSelectList node, Object data) {
Node parent = node.jjtGetParent();
checkEachChildOnNextLine(data, node, parent.getBeginLine(), parent.getBeginColumn() + 7);
return super.visit(node, data);
}
@Override
public Object visit(ASTBulkCollectIntoClause node, Object data) {
Node parent = node.jjtGetParent();
checkIndentation(data, node, parent.getBeginColumn() + indentation, "BULK COLLECT INTO");
checkEachChildOnNextLine(data, node, node.getBeginLine(), parent.getBeginColumn() + 7);
return super.visit(node, data);
}
@Override
public Object visit(ASTFromClause node, Object data) {
checkIndentation(data, node, node.jjtGetParent().getBeginColumn() + indentation, "FROM");
return super.visit(node, data);
}
@Override
public Object visit(ASTJoinClause node, Object data) {
// first child is the table reference
Node tableReference = node.jjtGetChild(0);
// remaining children are joins
int lineNumber = tableReference.getBeginLine();
for (int i = 1; i < node.jjtGetNumChildren(); i++) {
lineNumber++;
Node child = node.jjtGetChild(i);
if (child.getBeginLine() != lineNumber) {
addViolationWithMessage(data, child, child.getXPathNodeName() + " should be on line " + lineNumber);
}
List<ASTEqualityExpression> conditions = child.findDescendantsOfType(ASTEqualityExpression.class);
if (conditions.size() == 1) {
// one condition should be on the same line
ASTEqualityExpression singleCondition = conditions.get(0);
if (singleCondition.getBeginLine() != lineNumber) {
addViolationWithMessage(data, child,
"Join condition \"" + singleCondition.getImage() + "\" should be on line " + lineNumber);
}
} else {
// each condition on a separate line
for (ASTEqualityExpression singleCondition : conditions) {
lineNumber++;
if (singleCondition.getBeginLine() != lineNumber) {
addViolationWithMessage(data, child,
"Join condition \"" + singleCondition.getImage() + "\" should be on line "
+ lineNumber);
}
}
}
}
return super.visit(node, data);
}
@Override
public Object visit(ASTSubqueryOperation node, Object data) {
// get previous sibling
int thisIndex = node.jjtGetChildIndex();
Node prevSibling = node.jjtGetParent().jjtGetChild(thisIndex - 1);
checkIndentation(data, node, prevSibling.getBeginColumn(), node.getImage());
// it should also be on the next line
if (node.getBeginLine() != prevSibling.getEndLine() + 1) {
addViolationWithMessage(data, node,
node.getImage() + " should be on line " + (prevSibling.getEndLine() + 1));
}
return super.visit(node, data);
}
private int checkEachChildOnNextLine(Object data, Node parent, int firstLine, int indentation) {
int currentLine = firstLine;
for (int i = 0; i < parent.jjtGetNumChildren(); i++) {
Node child = parent.jjtGetChild(i);
if (child.getBeginLine() != currentLine) {
addViolationWithMessage(data, child, child.getImage() + " should be on line " + currentLine);
} else if (i > 0 && child.getBeginColumn() != indentation) {
addViolationWithMessage(data, child, child.getImage() + " should begin at column " + indentation);
}
// next entry needs to be on the next line
currentLine++;
}
return currentLine;
}
private void checkLineAndIndentation(Object data, Node node, int line, int indentation, String name) {
if (node.getBeginLine() != line) {
addViolationWithMessage(data, node, name + " should be on line " + line);
} else if (node.getBeginColumn() != indentation) {
addViolationWithMessage(data, node, name + " should begin at column " + indentation);
}
}
private void checkIndentation(Object data, Node node, int indentation, String name) {
if (node.getBeginColumn() != indentation) {
addViolationWithMessage(data, node, name + " should begin at column " + indentation);
}
}
@Override
public Object visit(ASTFormalParameters node, Object data) {
int parameterIndentation = node.jjtGetParent().getBeginColumn() + indentation;
checkEachChildOnNextLine(data, node, node.getBeginLine() + 1, parameterIndentation);
// check the data type alignment
List<ASTFormalParameter> parameters = node.findChildrenOfType(ASTFormalParameter.class);
if (parameters.size() > 1) {
ASTDatatype first = parameters.get(0).getFirstChildOfType(ASTDatatype.class);
for (int i = 1; first != null && i < parameters.size(); i++) {
ASTDatatype nextType = parameters.get(i).getFirstChildOfType(ASTDatatype.class);
if (nextType != null) {
checkIndentation(data, nextType, first.getBeginColumn(), "Type " + nextType.getImage());
}
}
}
return super.visit(node, data);
}
@Override
public Object visit(ASTDeclarativeSection node, Object data) {
int variableIndentation = node.getNthParent(2).getBeginColumn() + 2 * indentation;
int line = node.getBeginLine();
List<ASTVariableOrConstantDeclarator> variables = node
.findDescendantsOfType(ASTVariableOrConstantDeclarator.class);
int datatypeIndentation = variableIndentation;
if (!variables.isEmpty()) {
ASTDatatype datatype = variables.get(0).getFirstChildOfType(ASTDatatype.class);
if (datatype != null) {
datatypeIndentation = datatype.getBeginColumn();
}
}
for (ASTVariableOrConstantDeclarator variable : variables) {
checkLineAndIndentation(data, variable, line, variableIndentation, variable.getImage());
ASTDatatype datatype = variable.getFirstChildOfType(ASTDatatype.class);
if (datatype != null) {
checkIndentation(data, datatype, datatypeIndentation, "Type " + datatype.getImage());
}
line++;
}
return super.visit(node, data);
}
@Override
public Object visit(ASTArgumentList node, Object data) {
List<ASTArgument> arguments = node.findChildrenOfType(ASTArgument.class);
if (node.getEndColumn() > 120) {
addViolationWithMessage(data, node, "Line is too long, please split parameters on separate lines");
return super.visit(node, data);
}
if (arguments.size() > 3) {
// procedure calls with more than 3 parameters should use named parameters
if (usesSimpleParameters(arguments)) {
addViolationWithMessage(data, node,
"Procedure call with more than three parameters should use named parameters.");
}
// more than three parameters -> each parameter on a separate line
int line = node.getBeginLine();
int indentation = node.getBeginColumn();
int longestParameterEndColumn = 0;
for (ASTArgument argument : arguments) {
checkLineAndIndentation(data, argument, line, indentation, "Parameter " + argument.getImage());
line++;
if (argument.jjtGetChild(0) instanceof ASTUnqualifiedID) {
if (argument.jjtGetChild(0).getEndColumn() > longestParameterEndColumn) {
longestParameterEndColumn = argument.jjtGetChild(0).getEndColumn();
}
}
}
// now check for the indentation of the expressions
int expectedBeginColumn = longestParameterEndColumn + 3 + "=> ".length();
// take the indentation from the first one, if it is greater
if (!arguments.isEmpty() && arguments.get(0).jjtGetNumChildren() == 2
&& arguments.get(0).jjtGetChild(1).getBeginColumn() > expectedBeginColumn) {
expectedBeginColumn = arguments.get(0).jjtGetChild(1).getBeginColumn();
}
for (ASTArgument argument : arguments) {
if (argument.jjtGetNumChildren() == 2 && argument.jjtGetChild(0) instanceof ASTUnqualifiedID) {
Node expr = argument.jjtGetChild(1);
checkIndentation(data, expr, expectedBeginColumn, expr.getImage());
}
}
// closing paranthesis should be on a new line
Node primaryExpression = node.getNthParent(3);
if (primaryExpression.getEndLine() != node.getEndLine() + 1) {
addViolationWithMessage(data, primaryExpression, "Closing paranthesis should be on a new line.");
}
}
return super.visit(node, data);
}
private boolean usesSimpleParameters(List<ASTArgument> arguments) {
for (ASTArgument argument : arguments) {
if (argument.jjtGetNumChildren() == 1) {
return true;
}
}
return false;
}
}

View File

@ -9,6 +9,71 @@
Rules which enforce a specific coding style.
</description>
<rule name="CodeFormat"
language="plsql"
since="6.9.0"
message="Please check the formatting/indentation"
class="net.sourceforge.pmd.lang.plsql.rule.codestyle.CodeFormatRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_plsql_codestyle.html#codeformat">
<description>
This rule verifies that the PLSQL code is properly formatted. The following checks are executed:
SQL Queries:
* The selected columns must be each on a new line
* The keywords (BULK COLLECT INTO, FROM) start on a new line and are indented by one level
* UNION should be on the same indentation level as SELECT
* Each JOIN is on a new line. If there are more than one JOIN conditions, then each condition should be
on a separate line.
Parameter definitions for procedures:
* Each parameter should be on a new line
* Variable names as well as their types should be aligned
Variable declarations:
* Each variable should be on a new line
* Variable names as well as their types should be aligned
Calling a procedure:
* If there are more than 3 parameters
* then named parameters should be used
* and each parameter should be on a new line
</description>
<priority>3</priority>
<example><![CDATA[
BEGIN
-- select columns each on a separate line
SELECT cmer_id
,version
,cmp_id
BULK COLLECT INTO v_cmer_ids
,v_versions
,v_cmp_ids
FROM cmer;
-- each parameter on a new line
PROCEDURE create_prospect(
company_info_in IN prospects.company_info%TYPE -- Organization
,firstname_in IN persons.firstname%TYPE -- FirstName
,lastname_in IN persons.lastname%TYPE -- LastName
);
-- more than three parameters, each parameter on a separate line
webcrm_marketing.prospect_ins(
cmp_id_in => NULL
,company_info_in => company_info_in
,firstname_in => firstname_in
,lastname_in => lastname_in
,slt_code_in => NULL
);
END;
]]></example>
</rule>
<rule name="MisplacedPragma"
language="plsql"
since="5.5.2"

View File

@ -0,0 +1,11 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.plsql.rule.codestyle;
import net.sourceforge.pmd.testframework.PmdRuleTst;
public class CodeFormatTest extends PmdRuleTst {
// no additional unit tests
}