Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RedundantDo rule #91

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}