Swap the verbosity mechanism for ConformancePatternRule from an argument to a global switch.
PiperOrigin-RevId: 249425522
diff --git a/internal/tsetse/rules/conformance_pattern_rule.ts b/internal/tsetse/rules/conformance_pattern_rule.ts
index ec3c8bf..c78f90c 100644
--- a/internal/tsetse/rules/conformance_pattern_rule.ts
+++ b/internal/tsetse/rules/conformance_pattern_rule.ts
@@ -20,16 +20,14 @@
private readonly engine: PatternEngine<P>;
- constructor(
- config: Config<P>, fixer?: Fixer<MatchedNodeTypes[P]>,
- verbose?: boolean) {
+ constructor(config: Config<P>, fixer?: Fixer<MatchedNodeTypes[P]>) {
// TODO(rjamet): This cheats a bit with the typing, as TS doesn't realize
// that P is Config.kind.
// tslint:disable-next-line:no-any See above.
let engine: PatternEngine<any>;
switch (config.kind) {
case PatternKind.BANNED_PROPERTY_WRITE:
- engine = new PropertyWriteEngine(config, fixer, verbose);
+ engine = new PropertyWriteEngine(config, fixer);
break;
default:
throw new Error('Config type not recognized, or not implemented yet.');
diff --git a/internal/tsetse/tests/ban_conformance_pattern/util_test.ts b/internal/tsetse/tests/ban_conformance_pattern/util_test.ts
new file mode 100644
index 0000000..0588b47
--- /dev/null
+++ b/internal/tsetse/tests/ban_conformance_pattern/util_test.ts
@@ -0,0 +1,37 @@
+/**
+ * @fileoverview Tests for the various utilities that are not tied to a single
+ * rule.
+ */
+
+import 'jasmine';
+import {ConformancePatternRule, PatternKind} from '../../rules/conformance_pattern_rule';
+import {setDebug} from '../../util/ast_tools';
+import {compileAndCheck} from '../../util/testing/test_support';
+
+describe('Debug output', () => {
+ it('turns on and off', () => {
+ const source = `const obj = {prop:'val'}; obj.prop = 'foo';`;
+ const rule = new ConformancePatternRule({
+ errorMessage: 'does not matter',
+ kind: PatternKind.BANNED_PROPERTY_WRITE,
+ values: ['HTMLQuoteElement.prototype.cite']
+ });
+
+ const logs: string[] = [];
+ const realConsoleLog = console.log;
+ spyOn(console, 'log').and.callFake((s: string) => {
+ logs.push(s);
+ realConsoleLog(s);
+ });
+ setDebug(true);
+ compileAndCheck(rule, source);
+
+ expect(logs).toEqual([`inspecting obj.prop = 'foo'`]);
+
+ setDebug(false);
+ compileAndCheck(rule, source);
+
+ // Nothing more appended: debug was false
+ expect(logs).toEqual([`inspecting obj.prop = 'foo'`]);
+ });
+});
diff --git a/internal/tsetse/util/ast_tools.ts b/internal/tsetse/util/ast_tools.ts
index 44d26ee..cb0f0cb 100644
--- a/internal/tsetse/util/ast_tools.ts
+++ b/internal/tsetse/util/ast_tools.ts
@@ -6,6 +6,25 @@
import * as ts from 'typescript';
/**
+ * Triggers increased verbosity in the rules.
+ */
+let DEBUG = false;
+
+/**
+ * Turns on or off logging for ConformancePatternRules.
+ */
+export function setDebug(state: boolean) {
+ DEBUG = state;
+}
+
+/**
+ * Debug helper.
+ */
+export function debugLog(msg: string) {
+ if (DEBUG) console.log(msg);
+}
+
+/**
* Returns `n`'s parents in order.
*/
export function parents(n: ts.Node): ts.Node[] {
@@ -146,13 +165,6 @@
}
/**
- * Debug helper.
- */
-export function debugLog(verbose: boolean|undefined, msg: string) {
- if (verbose) console.info(msg);
-}
-
-/**
* If verbose, logs the given error that happened while walking n, with a
* stacktrace.
*/
@@ -163,7 +175,6 @@
} catch {
}
debugLog(
- verbose,
`Walking node ${nodeText} failed with error ${e}.\n` +
- `Stacktrace:\n${e.stack}`);
+ `Stacktrace:\n${e.stack}`);
}
diff --git a/internal/tsetse/util/fixer.ts b/internal/tsetse/util/fixer.ts
index 19de12a..f9b784e 100644
--- a/internal/tsetse/util/fixer.ts
+++ b/internal/tsetse/util/fixer.ts
@@ -52,20 +52,19 @@
*/
export function maybeAddNamedImport(
source: ts.SourceFile, importWhat: string, fromFile: string,
- importAs?: string, tazeComment?: string, v?: boolean): IndividualChange|
- undefined {
+ importAs?: string, tazeComment?: string): IndividualChange|undefined {
const importStatements = source.statements.filter(ts.isImportDeclaration);
const importSpecifier =
importAs ? `${importWhat} as ${importAs}` : importWhat;
for (const iDecl of importStatements) {
- const parsedDecl = maybeParseImportNode(iDecl, v);
+ const parsedDecl = maybeParseImportNode(iDecl);
if (!parsedDecl || parsedDecl.fromFile !== fromFile) {
// Not an import from the right file, or couldn't understand the import.
continue; // Jump to the next import.
}
if (ts.isNamespaceImport(parsedDecl.namedBindings)) {
- debugLog(v, `... but it's a wildcard import`);
+ debugLog(`... but it's a wildcard import`);
continue; // Jump to the next import.
}
@@ -78,12 +77,12 @@
iSpec.name.getText() === importWhat); // import {foo}
if (foundRightImport) {
- debugLog(v, `"${iDecl.getFullText()}" imports ${importWhat} as we want.`);
+ debugLog(`"${iDecl.getFullText()}" imports ${importWhat} as we want.`);
return; // Our request is already imported under the right name.
}
// Else, insert our symbol in the list of imports from that file.
- debugLog(v, `No named imports from that file, generating new fix`);
+ debugLog(`No named imports from that file, generating new fix`);
return {
start: parsedDecl.namedBindings.elements[0].getStart(),
end: parsedDecl.namedBindings.elements[0].getStart(),
@@ -117,27 +116,27 @@
*/
export function maybeAddNamespaceImport(
source: ts.SourceFile, fromFile: string, importAs: string,
- tazeComment?: string, v?: boolean): IndividualChange|undefined {
+ tazeComment?: string): IndividualChange|undefined {
const importStatements = source.statements.filter(ts.isImportDeclaration);
const hasTheRightImport = importStatements.some(iDecl => {
- const parsedDecl = maybeParseImportNode(iDecl, v);
+ const parsedDecl = maybeParseImportNode(iDecl);
if (!parsedDecl || parsedDecl.fromFile !== fromFile) {
// Not an import from the right file, or couldn't understand the import.
return false;
}
- debugLog(v, `"${iDecl.getFullText()}" is an import from the right file`);
+ debugLog(`"${iDecl.getFullText()}" is an import from the right file`);
if (ts.isNamedImports(parsedDecl.namedBindings)) {
- debugLog(v, `... but it's a named import`);
+ debugLog(`... but it's a named import`);
return false; // irrelevant to our namespace imports
}
// Else, bindings is a NamespaceImport.
if (parsedDecl.namedBindings.name.getText() !== importAs) {
- debugLog(v, `... but not the right name, we need to reimport`);
+ debugLog(`... but not the right name, we need to reimport`);
return false;
}
- debugLog(v, `... and the right name, no need to reimport`);
+ debugLog(`... and the right name, no need to reimport`);
return true;
});
@@ -162,14 +161,13 @@
* parts, undefined if the import declaration is valid but not understandable by
* the checker.
*/
-function maybeParseImportNode(iDecl: ts.ImportDeclaration, v?: boolean): {
+function maybeParseImportNode(iDecl: ts.ImportDeclaration): {
namedBindings: ts.NamedImportBindings|ts.NamespaceImport,
fromFile: string
}|undefined {
if (!iDecl.importClause) {
// something like import "./file";
- debugLog(
- v, `Ignoring import without imported symbol: ${iDecl.getFullText()}`);
+ debugLog(`Ignoring import without imported symbol: ${iDecl.getFullText()}`);
return;
}
if (iDecl.importClause.name || !iDecl.importClause.namedBindings) {
@@ -177,11 +175,11 @@
// Not much we can do with that when trying to get a hold of some symbols,
// so just ignore that line (worst case, we'll suggest another import
// style).
- debugLog(v, `Ignoring import: ${iDecl.getFullText()}`);
+ debugLog(`Ignoring import: ${iDecl.getFullText()}`);
return;
}
if (!ts.isStringLiteral(iDecl.moduleSpecifier)) {
- debugLog(v, `Ignoring import whose module specifier is not literal`);
+ debugLog(`Ignoring import whose module specifier is not literal`);
return;
}
return {
diff --git a/internal/tsetse/util/match_symbol.ts b/internal/tsetse/util/match_symbol.ts
index 1a05c72..5df53b1 100644
--- a/internal/tsetse/util/match_symbol.ts
+++ b/internal/tsetse/util/match_symbol.ts
@@ -35,23 +35,23 @@
}
}
- matches(n: ts.Node, tc: ts.TypeChecker, verbose?: boolean): boolean {
+ matches(n: ts.Node, tc: ts.TypeChecker): boolean {
// Get the symbol (or the one at the other end of this alias) that we're
// looking at.
const s = dealias(tc.getSymbolAtLocation(n), tc);
if (!s) {
- debugLog(verbose, `cannot get symbol`);
+ debugLog(`cannot get symbol`);
return false;
}
// The TS-provided FQN tells us the full identifier, and the origin file
// in some circumstances.
const fqn = tc.getFullyQualifiedName(s);
- debugLog(verbose, `got FQN ${fqn}`);
+ debugLog(`got FQN ${fqn}`);
// Name-based check
if (!(fqn.endsWith('.' + this.bannedName) || fqn === this.bannedName)) {
- debugLog(verbose, `FQN ${fqn} doesn't match name ${this.bannedName}`);
+ debugLog(`FQN ${fqn} doesn't match name ${this.bannedName}`);
return false; // not a use of the symbols we want
}
@@ -61,7 +61,7 @@
// and bad fixes.
const p = n.parent;
if (p && (isDeclaration(p) || isPartOfImportStatement(p))) {
- debugLog(verbose, `We don't flag symbol declarations`);
+ debugLog(`We don't flag symbol declarations`);
return false;
}
@@ -74,18 +74,17 @@
// We need to trace things back, so get declarations of the symbol.
const declarations = s.getDeclarations();
if (!declarations) {
- debugLog(verbose, `Symbol never declared?`);
+ debugLog(`Symbol never declared?`);
return false;
}
if (!declarations.some(isAmbientDeclaration) &&
!declarations.some(isInStockLibraries)) {
- debugLog(
- verbose, `Symbol neither ambient nor from the stock libraries`);
+ debugLog(`Symbol neither ambient nor from the stock libraries`);
return false;
}
}
- debugLog(verbose, `all clear, report finding`);
+ debugLog(`all clear, report finding`);
return true;
}
}
@@ -123,8 +122,7 @@
/**
* @param n The PropertyAccessExpression we're looking at.
*/
- matches(
- n: ts.PropertyAccessExpression, tc: ts.TypeChecker, verbose?: boolean) {
+ matches(n: ts.PropertyAccessExpression, tc: ts.TypeChecker) {
return n.name.text === this.bannedProperty &&
this.typeMatches(tc.getTypeAtLocation(n.expression));
}
diff --git a/internal/tsetse/util/pattern_engines/pattern_engine.ts b/internal/tsetse/util/pattern_engines/pattern_engine.ts
index 4d315b5..5dbbddc 100644
--- a/internal/tsetse/util/pattern_engines/pattern_engine.ts
+++ b/internal/tsetse/util/pattern_engines/pattern_engine.ts
@@ -8,8 +8,7 @@
export abstract class PatternEngine<P extends PatternKind> {
constructor(
protected readonly config: Config<P>,
- protected readonly fixer?: Fixer<MatchedNodeTypes[P]>,
- protected readonly verbose?: boolean) {}
+ protected readonly fixer?: Fixer<MatchedNodeTypes[P]>) {}
abstract register(checker: Checker): void;
}
diff --git a/internal/tsetse/util/pattern_engines/property_write_engine.ts b/internal/tsetse/util/pattern_engines/property_write_engine.ts
index ea8fe35..7af7ccf 100644
--- a/internal/tsetse/util/pattern_engines/property_write_engine.ts
+++ b/internal/tsetse/util/pattern_engines/property_write_engine.ts
@@ -16,9 +16,8 @@
private readonly matcher: PropertyMatcher;
constructor(
config: Config<PatternKind.BANNED_PROPERTY_WRITE>,
- fixer?: Fixer<MatchedNodeTypes[PatternKind.BANNED_PROPERTY_WRITE]>,
- verbose?: boolean) {
- super(config, fixer, verbose);
+ fixer?: Fixer<MatchedNodeTypes[PatternKind.BANNED_PROPERTY_WRITE]>) {
+ 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) {
@@ -39,8 +38,8 @@
!isPropertyWriteExpression(n)) {
return;
}
- debugLog(this.verbose, `inspecting ${n.getFullText().trim()}`);
- if (this.matcher.matches(n.left, c.typeChecker, this.verbose)) {
+ debugLog(`inspecting ${n.getFullText().trim()}`);
+ if (this.matcher.matches(n.left, c.typeChecker)) {
const fix: Fix|undefined =
this.fixer ? this.fixer.getFixForFlaggedNode(n) : undefined;
c.addFailureAtNode(n, this.config.errorMessage, fix);
