Add a property_write_non_constant engine to Tsetse's ConformancePattern.
Same as property_write, but only triggers on writing non-constant values.
PiperOrigin-RevId: 249633880
diff --git a/internal/tsetse/rules/conformance_pattern_rule.ts b/internal/tsetse/rules/conformance_pattern_rule.ts
index c78f90c..b683355 100644
--- a/internal/tsetse/rules/conformance_pattern_rule.ts
+++ b/internal/tsetse/rules/conformance_pattern_rule.ts
@@ -4,6 +4,7 @@
import {Fixer} from '../util/fixer';
import {Config, MatchedNodeTypes, PatternKind} from '../util/pattern_config';
import {PatternEngine} from '../util/pattern_engines/pattern_engine';
+import {PropertyNonConstantWriteEngine} from '../util/pattern_engines/property_non_constant_write_engine';
import {PropertyWriteEngine} from '../util/pattern_engines/property_write_engine';
/**
@@ -27,7 +28,13 @@
let engine: PatternEngine<any>;
switch (config.kind) {
case PatternKind.BANNED_PROPERTY_WRITE:
- engine = new PropertyWriteEngine(config, fixer);
+ engine = new PropertyWriteEngine(
+ config as Config<PatternKind.BANNED_PROPERTY_WRITE>, fixer);
+ break;
+ case PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE:
+ engine = new PropertyNonConstantWriteEngine(
+ config as Config<PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE>,
+ fixer);
break;
default:
throw new Error('Config type not recognized, or not implemented yet.');
diff --git a/internal/tsetse/tests/ban_conformance_pattern/property_non_constant_write_test.ts b/internal/tsetse/tests/ban_conformance_pattern/property_non_constant_write_test.ts
new file mode 100644
index 0000000..8b51796
--- /dev/null
+++ b/internal/tsetse/tests/ban_conformance_pattern/property_non_constant_write_test.ts
@@ -0,0 +1,27 @@
+import 'jasmine';
+import {ConformancePatternRule, PatternKind} from '../../rules/conformance_pattern_rule';
+import {compileAndCheck, customMatchers} from '../../util/testing/test_support';
+
+describe('BANNED_PROPERTY_NON_CONSTANT_WRITE', () => {
+ it('matches a trivial example', () => {
+ const source = `const q = document.createElement('q');\n` +
+ `q.cite = 'some example string';\n` + // literal
+ `q.cite = window.name;\n`; // non-literal
+ const rule = new ConformancePatternRule({
+ errorMessage: 'do not cite dynamically',
+ kind: PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE,
+ values: ['HTMLQuoteElement.prototype.cite']
+ });
+ const results = compileAndCheck(rule, source);
+
+ expect(results.length).toBe(1);
+ expect(results[0])
+ .toBeFailureMatching(
+ {start: 71, end: 91, errorMessage: 'do not cite dynamically'});
+ });
+});
+
+
+beforeEach(() => {
+ jasmine.addMatchers(customMatchers);
+});
diff --git a/internal/tsetse/tests/ban_conformance_pattern/util_test.ts b/internal/tsetse/tests/ban_conformance_pattern/util_test.ts
index 0588b47..94ab234 100644
--- a/internal/tsetse/tests/ban_conformance_pattern/util_test.ts
+++ b/internal/tsetse/tests/ban_conformance_pattern/util_test.ts
@@ -5,8 +5,9 @@
import 'jasmine';
import {ConformancePatternRule, PatternKind} from '../../rules/conformance_pattern_rule';
-import {setDebug} from '../../util/ast_tools';
-import {compileAndCheck} from '../../util/testing/test_support';
+import {isInStockLibraries, setDebug} from '../../util/ast_tools';
+import {isLiteral} from '../../util/is_literal';
+import {compile, compileAndCheck} from '../../util/testing/test_support';
describe('Debug output', () => {
it('turns on and off', () => {
@@ -35,3 +36,62 @@
expect(logs).toEqual([`inspecting obj.prop = 'foo'`]);
});
});
+
+describe('The constant-ness logic', () => {
+ it('understands constants', () => {
+ // Keep these to single-expression programs.
+ const constantExpressionsSources = [
+ `'hello'`,
+ `'hello' + 'hi'`,
+ `1`,
+ `1 + 1`,
+ '`abcdef`',
+ '`abcd${"ef"}`',
+ '`abcd${1+1}`+`hi`',
+ `1 ? 'hi' : 'hello'`,
+ `window.name ? 'hi' : 'hello'`,
+ ];
+
+ // We don't bother with a rule for this one.
+ const constantProgram = compile(...constantExpressionsSources);
+ const constantCompiledSources = constantProgram.getSourceFiles();
+ const constantTc = constantProgram.getTypeChecker();
+ const constantExpressions =
+ constantCompiledSources.filter(s => !isInStockLibraries(s))
+ .map(s => s.statements[0].getChildren()[0]);
+
+ for (const expr of constantExpressions) {
+ expect(isLiteral(constantTc, expr))
+ .toBe(
+ true,
+ `Expected "${expr.getFullText()}" to be considered constant.`);
+ }
+ });
+
+
+ it('understands non-constants', () => {
+ const nonconstantExpressionsSources = [
+ `window.name`,
+ `'hello' + window.name`,
+ `window.name + 'hello'`,
+ '`abcd${window.name}`',
+ `1 ? window.name : 'hello'`,
+ `1 ? 'hello' : window.name`,
+ ];
+
+ const nonconstantProgram = compile(...nonconstantExpressionsSources);
+ const nonconstantCompiledSources = nonconstantProgram.getSourceFiles();
+ const nonconstantTc = nonconstantProgram.getTypeChecker();
+ const nonconstantExpressions =
+ nonconstantCompiledSources.filter(s => !isInStockLibraries(s))
+ .map(s => s.statements[0].getChildren()[0]);
+
+ for (const expr of nonconstantExpressions) {
+ expect(isLiteral(nonconstantTc, expr))
+ .toBe(
+ false,
+ `Expected "${
+ expr.getFullText()}" not to be considered constant.`);
+ }
+ });
+});
diff --git a/internal/tsetse/util/pattern_config.ts b/internal/tsetse/util/pattern_config.ts
index c2748d9..76e2d71 100644
--- a/internal/tsetse/util/pattern_config.ts
+++ b/internal/tsetse/util/pattern_config.ts
@@ -7,6 +7,7 @@
*/
export enum PatternKind {
BANNED_PROPERTY_WRITE = 'banned-property-write',
+ BANNED_PROPERTY_NON_CONSTANT_WRITE = 'banned-property-non-constant-write'
}
/**
@@ -30,4 +31,7 @@
[PatternKind.BANNED_PROPERTY_WRITE]: ts.BinaryExpression&{
left: ts.PropertyAccessExpression;
};
+ [PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE]: ts.BinaryExpression&{
+ left: ts.PropertyAccessExpression;
+ };
}
diff --git a/internal/tsetse/util/pattern_engines/property_non_constant_write_engine.ts b/internal/tsetse/util/pattern_engines/property_non_constant_write_engine.ts
new file mode 100644
index 0000000..9e26cd7
--- /dev/null
+++ b/internal/tsetse/util/pattern_engines/property_non_constant_write_engine.ts
@@ -0,0 +1,58 @@
+import * as ts from 'typescript';
+import {Checker} from '../../checker';
+import {ErrorCode} from '../../error_code';
+import {Fix} from '../../failure';
+import {debugLog, isPropertyWriteExpression, shouldExamineNode} from '../ast_tools';
+import {Fixer} from '../fixer';
+import {isLiteral} from '../is_literal';
+import {PropertyMatcher} from '../match_symbol';
+import {Config, MatchedNodeTypes, PatternKind} from '../pattern_config';
+import {PatternEngine} from './pattern_engine';
+
+// Just for conciseness.
+type BanKind = PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE;
+
+/**
+ * The engine for BANNED_PROPERTY_NON_CONSTANT_WRITE.
+ */
+export class PropertyNonConstantWriteEngine extends PatternEngine<BanKind> {
+ private readonly matcher: PropertyMatcher;
+ constructor(
+ config: Config<BanKind>, fixer?: Fixer<MatchedNodeTypes[BanKind]>) {
+ super(config, fixer);
+ // TODO: Support more than one single value here, or even build a
+ // multi-pattern engine. This would help for performance.
+ if (this.config.values.length !== 1) {
+ throw new Error(
+ `BANNED_PROPERTY_NON_CONSTANT_WRITE expects one value, got(${
+ this.config.values.join(',')})`);
+ }
+ this.matcher = PropertyMatcher.fromSpec(this.config.values[0]);
+ }
+
+ register(checker: Checker) {
+ checker.on(
+ ts.SyntaxKind.BinaryExpression, this.check.bind(this),
+ ErrorCode.CONFORMANCE_PATTERN);
+ }
+
+ check(c: Checker, n: ts.BinaryExpression) {
+ if (!shouldExamineNode(n) || n.getSourceFile().isDeclarationFile ||
+ !isPropertyWriteExpression(n)) {
+ return;
+ }
+ debugLog(`inspecting ${n.getFullText().trim()}`);
+ if (!this.matcher.matches(n.left, c.typeChecker)) {
+ debugLog('Not an assignment to the right property');
+ return;
+ }
+ if (isLiteral(c.typeChecker, n.right)) {
+ debugLog(`Assigned value (${
+ n.right.getFullText()}) is a compile-time constant.`);
+ return;
+ }
+ const fix: Fix|undefined =
+ this.fixer ? this.fixer.getFixForFlaggedNode(n) : undefined;
+ c.addFailureAtNode(n, this.config.errorMessage, fix);
+ }
+}
diff --git a/internal/tsetse/util/testing/test_support.ts b/internal/tsetse/util/testing/test_support.ts
index 010ad8c..8adb90a 100644
--- a/internal/tsetse/util/testing/test_support.ts
+++ b/internal/tsetse/util/testing/test_support.ts
@@ -10,7 +10,13 @@
import {AbstractRule} from '../../rule';
-function compile(...sourceCode: string[]): ts.Program {
+
+/**
+ * Turns the provided source (as strings) into a ts.Program. The source files
+ * will be named `.../file_${n}.ts`, with n the index of the source file in
+ * the `sourceCode` array.
+ */
+export function compile(...sourceCode: string[]): ts.Program {
const temporaryFolder = os.tmpdir() +
`/tslint_test_input_${crypto.randomBytes(16).toString('hex')}`;
const fullPaths: string[] = [];
