Skip to content

Commit

Permalink
RedundantDo rule
Browse files Browse the repository at this point in the history
  • Loading branch information
Venkat Raju authored and Venkat Raju committed Jul 26, 2019
1 parent 839bd4c commit 86e9e8a
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 7 deletions.
46 changes: 46 additions & 0 deletions __tests__/files/ZTestRedundantDO.PROC
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
public void method1()
/*

*/
do {
write "something"
}

type Boolean x = false

// no quit
if x do {

}
type Number abcd =123
// with quit
if x do { quit:abcd>100

}
else if abcd=100 do {

}
else do {

}

while abcd<100 do {

}
// with post conditional quit
while abcd<100 do { quit:abcd=88

}

type Number i
// conventional
for i=0:1:10 do {

}

type String dummy = ""
for set dummy = abcd(dummy).order() quit:dummy.isNull() do {

}

quit
67 changes: 67 additions & 0 deletions __tests__/redundantDo-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import * as api from '../src/pslLint/api';
import * as utils from './ruleUtils';
import { RedundantDoStatement } from '../src/pslLint/cli/lib/pslLint/doBlock';

describe('Members tests', () => {
let redundantDiagnostics: api.Diagnostic[] = [];


beforeAll(async () => {
redundantDiagnostics = await utils.getDiagnostics('ZTestRedundantDO.PROC', RedundantDoStatement.name);
});

test('block as first statement', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(3, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(0);
});

test('if block with no quit', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(11, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "if"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('if block with quit', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(16, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "if"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('else if block', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(19, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "if"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('else block ', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(22, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "else"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('while block', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(26, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "while"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('while block with post quit', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(30, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "while"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('conventional for', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(36, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "for"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});
test('for order', () => {
const diagnosticsOnLine = utils.diagnosticsOnLine(41, redundantDiagnostics);
expect(diagnosticsOnLine.length).toBe(1);
expect(diagnosticsOnLine[0].message).toBe(`"do" statement on same line as "quit"`);
expect(diagnosticsOnLine[0].severity).toBe(api.DiagnosticSeverity.Warning);
});

});
19 changes: 19 additions & 0 deletions __tests__/statementParser-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,25 @@ describe('recursive tests', () => {
const setStatement = statements[1];
const equal = setStatement.expressions[0] as BinaryOperator;
expect(equal.kind).toBe(SyntaxKind.ASSIGNMENT);
const quitStatement = statements[2];
expect(quitStatement.kind).toBe(SyntaxKind.QUIT_STATEMENT);
expect(quitStatement.expressions.length).toBe(1);
const doStatement = statements[3];
expect(doStatement.kind).toBe(SyntaxKind.DO_STATEMENT);
expect(doStatement.expressions.length).toBe(1);
});
test('for order empty quit', () => {
const parser = parse('for set seq=tras(seq).order() quit do f(tras(seq))');
const statements = parser.parseLine();
const setStatement = statements[1];
const equal = setStatement.expressions[0] as BinaryOperator;
expect(equal.kind).toBe(SyntaxKind.ASSIGNMENT);
const quitStatement = statements[2];
expect(quitStatement.kind).toBe(SyntaxKind.QUIT_STATEMENT);
expect(quitStatement.expressions.length).toBe(0);
const doStatement = statements[3];
expect(doStatement.kind).toBe(SyntaxKind.DO_STATEMENT);
expect(doStatement.expressions.length).toBe(1);
});
test('ret identifier', () => {
const parser = parse('set ret.x = y');
Expand Down
22 changes: 15 additions & 7 deletions src/parser/statementParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,15 @@ export interface PostCondition extends Node {
export type Expression = Value | BinaryOperator | PostCondition | MultiSet;

export interface Statement extends Node {
/**
* The type of the statement, i.e. `do`, `if`, `set`, etc.
* For the statement `set x = 1`, `set` is the action and `x = 1` is the expression.
*/
action: Token;
/**
* The code following the `action`. For the statement `set x = 1`,
* `set` is the action and `x = 1` is the expression.
*/
expressions: Expression[];
block?: Node;
}
Expand Down Expand Up @@ -215,8 +223,8 @@ export class StatementParser {
let loadFunction: () => Expression | undefined;
let kind: SyntaxKind;

const loadSingleExpression = (): Statement => {
if (!this.next(true)) return { action, kind, expressions: [] };
const loadSingleExpression = (skipSpaces: boolean): Statement => {
if (!this.next(skipSpaces)) return { action, kind, expressions: [] };
const expression: Expression | undefined = loadFunction();
const expressions = expression ? [expression] : [];
return { kind, action, expressions };
Expand Down Expand Up @@ -247,25 +255,25 @@ export class StatementParser {
case STATEMENT_KEYWORD.CATCH:
loadFunction = () => this.parseExpression();
kind = SyntaxKind.CATCH_STATEMENT;
return loadCommaSeparatedExpressions();
return loadSingleExpression(true);

case STATEMENT_KEYWORD.FOR:
return this.parseForStatement();

case STATEMENT_KEYWORD.QUIT:
loadFunction = () => this.parseExpression();
kind = SyntaxKind.QUIT_STATEMENT;
return loadCommaSeparatedExpressions();
return loadSingleExpression(false);

case STATEMENT_KEYWORD.RETURN:
loadFunction = () => this.parseExpression();
kind = SyntaxKind.RETURN_STATEMENT;
return loadSingleExpression();
return loadSingleExpression(true);

case STATEMENT_KEYWORD.WHILE:
loadFunction = () => this.parseExpression();
kind = SyntaxKind.WHILE_STATEMENT;
return loadSingleExpression();
return loadSingleExpression(true);

case STATEMENT_KEYWORD.TYPE:
return this.parseTypeStatement();
Expand Down Expand Up @@ -596,7 +604,7 @@ export class StatementParser {
return this.parseValue(operator);
}

if (value && this.activeToken && this.activeToken.isWhiteSpace()) this.next(true);
if (value && this.activeToken && this.activeToken.isWhiteSpace()) this.next();

return value;
}
Expand Down
2 changes: 2 additions & 0 deletions src/pslLint/activate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { MethodParametersOnNewLine } from './parameters';
import { RuntimeStart } from './runtime';
import { TblColDocumentation } from './tblcolDoc';
import { TodoInfo } from './todos';
import { RedundantDoStatement } from './doBlock';

/**
* Add new rules here to have them checked at the appropriate time.
Expand All @@ -29,6 +30,7 @@ const fileDefinitionRules: FileDefinitionRule[] = [
];
const pslRules: PslRule[] = [
new TodoInfo(),
new RedundantDoStatement(),
];
const memberRules: MemberRule[] = [
new MemberCamelCase(),
Expand Down
36 changes: 36 additions & 0 deletions src/pslLint/doBlock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { PslRule, Diagnostic, DiagnosticSeverity } from "./api";
import { SyntaxKind, Statement } from "../parser";


// if x do {
// }

// if x do y

export class RedundantDoStatement extends PslRule {

report(): Diagnostic[] {
// for (const statement of this.parsedDocument.statements) {}
const diagnostics: Diagnostic[] = [];
for (const method of this.parsedDocument.methods) {
// const validStatementKinds = [SyntaxKind.FOR_STATEMENT, SyntaxKind.IF_STATEMENT, SyntaxKind.WHILE_STATEMENT];
let previousStatment: Statement = undefined;
for (const currentStatment of method.statements) {
if (currentStatment.kind === SyntaxKind.DO_STATEMENT && currentStatment.expressions.length === 0) {
const currentStatementLine = currentStatment.action.position.line;
if (!previousStatment) continue;
const previousStatmentLine = previousStatment.action.position.line;
if (currentStatementLine === previousStatmentLine) {
const diagnostic = new Diagnostic(currentStatment.action.getRange(), `"do" statement on same line as "${previousStatment.action.value}"`, this.ruleName, DiagnosticSeverity.Warning);
diagnostic.source = 'lint';
diagnostics.push(diagnostic);
}
}
previousStatment = currentStatment;
}
}


return diagnostics;
}
}

0 comments on commit 86e9e8a

Please sign in to comment.