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.

Sign up for GitHub

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

fix(CC001): correct parsing of multiline Conditions #402

Merged
merged 1 commit into from
Dec 5, 2023
Merged

fix(CC001): correct parsing of multiline Conditions #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 17 additions & 15 deletions lib/package/Condition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2019 Google LLC
Copyright 2019, 2023 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -14,14 +14,14 @@
limitations under the License.
*/

var TruthTable = require("./TruthTable.js");
const TruthTable = require("./TruthTable.js");

function Condition(element, parent) {
this.parent = parent;
this.element = element;
}

Condition.prototype.getExpression = function() {
Condition.prototype.getExpression = function () {
return (
(this.element.childNodes &&
this.element.childNodes[0] &&
Expand All @@ -30,51 +30,53 @@ Condition.prototype.getExpression = function() {
);
};

Condition.prototype.getMessages = function() {
Condition.prototype.getMessages = function () {
return this.parent.getMessages();
};

Condition.prototype.getElement = function() {
Condition.prototype.getElement = function () {
return this.element;
};

Condition.prototype.getParent = function() {
Condition.prototype.getParent = function () {
return this.parent;
};

Condition.prototype.getLines = function(start, stop) {
Condition.prototype.getLines = function (start, stop) {
return this.parent.getLines(start, stop);
};

Condition.prototype.getSource = function() {
Condition.prototype.getSource = function () {
if (!this.source) {
var start = this.element.lineNumber - 1,
const start = this.element.lineNumber - 1,
stop = this.element.nextSibling.lineNumber - 1;
this.source = this.getLines(start, stop);
}
return this.source;
};

Condition.prototype.getTruthTable = function() {
Condition.prototype.getTruthTable = function () {
if (!this.truthTable) {
this.truthTable = new TruthTable(this.getExpression());
this.truthTable = new TruthTable(
this.getExpression().replace(/(\n| +)/g, " ")
);
}
return this.truthTable;
};

Condition.prototype.addMessage = function(msg) {
Condition.prototype.addMessage = function (msg) {
if (!msg.hasOwnProperty("entity")) {
msg.entity = this;
}
this.parent.addMessage(msg);
};

Condition.prototype.onConditions = function(pluginFunction, cb) {
Condition.prototype.onConditions = function (pluginFunction, cb) {
pluginFunction(this, cb);
};

Condition.prototype.summarize = function() {
var summary = {};
Condition.prototype.summarize = function () {
const summary = {};
summary.type = "Condition";
summary.condition = this.getExpression();
try {
Expand Down
39 changes: 22 additions & 17 deletions lib/package/plugins/CC001-checkConditionForLiterals.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2019-2020 Google LLC
Copyright 2019-2023 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -14,27 +14,32 @@
limitations under the License.
*/

const ruleId = require("../myUtil.js").getRuleId(),
debug = require("debug")("apigeelint:" + ruleId);

const plugin = {
ruleId : require("../myUtil.js").getRuleId(),
name: "Literals in Conditionals",
message:
"Single term literals statically evaluate to True or False and needlessly complicate a conditional at best, at worst they create dead code or are misleading in implying condtional execution.",
fatal: false,
severity: 1, //warning
nodeType: "Condition",
enabled: true
};
ruleId,
name: "Literals in Conditionals",
message:
"Single term literals statically evaluate to True or False and needlessly complicate a conditional at best, at worst they create dead code or are misleading in implying condtional execution.",
fatal: false,
severity: 1, //warning
nodeType: "Condition",
enabled: true
};

const onCondition = function(condition, cb) {
condition.getTruthTable().getAST(function(ast) {
let hasLiteral = false,
nodes = [ast],
actions = [{ action: "root", parent: undefined }];
const onCondition = function (condition, cb) {
debug(`onCondition (${condition.getExpression()})`);
condition.getTruthTable().getAST(function (ast) {
let hasLiteral = false;
const nodes = [ast],
actions = [{ action: "root", parent: undefined }];

while (nodes[0] && !hasLiteral) {
let node = nodes.pop(),
const node = nodes.pop(),
parentAction = actions.pop();

debug(`node (${JSON.stringify(node)})`);
if (
node.type === "constant" &&
(!parentAction.parent ||
Expand All @@ -55,7 +60,7 @@ const onCondition = function(condition, cb) {
});
} else if (node.args) {
//lets also capture the prior action
let act = { action: node.action, parent: undefined };
const act = { action: node.action, parent: undefined };
if (parentAction && parentAction.action) {
act.parent = parentAction.action;
}
Expand Down
90 changes: 40 additions & 50 deletions test/specs/CC001-CheckConditionForLiterals.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2019-2021 Google LLC
Copyright 2019-2023 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -16,42 +16,35 @@
/* global it, describe */

const assert = require("assert"),
testID = "CC001",
debug = require("debug")("apigeelint:" + testID),
bl = require("../../lib/package/bundleLinter.js"),
Condition = require("../../lib/package/Condition.js"),
plugin = require(bl.resolvePlugin(testID)),
Dom = require("@xmldom/xmldom").DOMParser,
test = function(exp, expected) {
it(`tests whether exp(${exp}) includes a literal, expect(${expected})`,
function() {
let doc = new Dom().parseFromString(exp),
c = new Condition(doc, this);
testID = "CC001",
debug = require("debug")("apigeelint:" + testID),
bl = require("../../lib/package/bundleLinter.js"),
Condition = require("../../lib/package/Condition.js"),
plugin = require(bl.resolvePlugin(testID)),
Dom = require("@xmldom/xmldom").DOMParser,
test = function (exp, expected) {
it(`tests whether exp(${exp}) includes a literal, expect(${expected})`, function () {
const doc = new Dom().parseFromString(exp),
c = new Condition(doc, this);

c.addMessage = function(msg) {
debug(msg);
};
c.addMessage = function (msg) {
debug(msg);
};

plugin.onCondition(c, function(err, result) {
assert.equal(
err,
undefined,
err ? " err " : " no err"
);
assert.equal(
result,
expected,
result ? " literal found " : "literal not found"
);
});
}
plugin.onCondition(c, function (e, result) {
assert.equal(e, undefined, e ? " error " : " no error");
assert.equal(
result,
expected,
result ? " literal found " : "literal not found"
);
};
});
});
};

//now generate a full report and check the format of the report

describe(`${testID} - ${plugin.plugin.name}`, function() {

describe(`${testID} - ${plugin.plugin.name}`, function () {
test("false", true);
test("true", true);
test("true OR false", true);
Expand All @@ -64,39 +57,36 @@ describe(`${testID} - ${plugin.plugin.name}`, function() {
test("1", true);
test('"foo"', true);
test("request.queryparams.foo", false);
test('request.header.Content-Type = "application/json"', false);
test(
'request.header.Content-Type = "application/json"',
'request.verb = "POST" and request.header.Content-Type = "application/json"',
false
);
// the newlines in the following condition are important
test(
'request.verb = "POST" and request.header.Content-Type = "application/json"',
`request.header.content-type != "text/xml" AND
request.header.content-type != "application/xml`,
false
);
});

describe(`${testID} - Print plugin results`, function() {
describe(`${testID} - Print plugin results`, function () {
debug("test configuration: " + JSON.stringify(configuration));
const Bundle = require("../../lib/package/Bundle.js"),
bundle = new Bundle(configuration);
bundle = new Bundle(configuration);
bl.executePlugin(testID, bundle);
let report = bundle.getReport();

it("should create a report object with valid schema", function() {
const report = bundle.getReport();

let formatter = bl.getFormatter("json.js");
it("should create a report object with valid schema", function () {
const formatter = bl.getFormatter("json.js");
if (!formatter) {
assert.fail("formatter implementation not defined");
}
let schema = require("./../fixtures/reportSchema.js"),
Validator = require("jsonschema").Validator,
v = new Validator(),
jsonReport = JSON.parse(formatter(report)),
validationResult = v.validate(jsonReport, schema);
assert.equal(
validationResult.errors.length,
0,
validationResult.errors
);
const schema = require("./../fixtures/reportSchema.js"),
Validator = require("jsonschema").Validator,
v = new Validator(),
jsonReport = JSON.parse(formatter(report)),
validationResult = v.validate(jsonReport, schema);
assert.equal(validationResult.errors.length, 0, validationResult.errors);
});

});