Remove the templating layer for pattern engines.
This was a misguided attempt to provide more type safety that tried to bridge templating and enums, which rarely ends well. For simplicity's sake, there's no specificity in Fixer or PatternEngine types anymore: they now use ts.Node.
PiperOrigin-RevId: 257170333
diff --git a/internal/tsetse/rules/conformance_pattern_rule.ts b/internal/tsetse/rules/conformance_pattern_rule.ts
index 707fffc..677490d 100644
--- a/internal/tsetse/rules/conformance_pattern_rule.ts
+++ b/internal/tsetse/rules/conformance_pattern_rule.ts
@@ -2,7 +2,7 @@
import {ErrorCode} from '../error_code';
import {AbstractRule} from '../rule';
import {Fixer} from '../util/fixer';
-import {Config, MatchedNodeTypes, PatternKind} from '../util/pattern_config';
+import {Config, PatternKind} from '../util/pattern_config';
import {NameEngine} from '../util/pattern_engines/name_engine';
import {PatternEngine} from '../util/pattern_engines/pattern_engine';
import {PropertyNonConstantWriteEngine} from '../util/pattern_engines/property_non_constant_write_engine';
@@ -15,42 +15,27 @@
* This is templated, mostly to ensure the nodes that have been matched
* correspond to what the Fixer expects.
*/
-export class ConformancePatternRule<P extends keyof MatchedNodeTypes> implements
- AbstractRule {
+export class ConformancePatternRule implements AbstractRule {
readonly ruleName: string;
readonly code = ErrorCode.CONFORMANCE_PATTERN;
- private readonly engine: PatternEngine<P>;
+ private readonly engine: PatternEngine;
- 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>;
- // Formatter breaks the types, b/135552145
- // clang-format off
+ constructor(config: Config, fixer?: Fixer) {
switch (config.kind) {
case PatternKind.BANNED_PROPERTY_WRITE:
- engine = new PropertyWriteEngine(
- config as Config<PatternKind.BANNED_PROPERTY_WRITE>,
- fixer as Fixer<MatchedNodeTypes[PatternKind.BANNED_PROPERTY_WRITE]>);
+ this.engine = new PropertyWriteEngine(config, fixer);
break;
case PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE:
- engine = new PropertyNonConstantWriteEngine(
- config as Config<PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE>,
- fixer as Fixer<MatchedNodeTypes[PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE]>);
+ this.engine = new PropertyNonConstantWriteEngine(config, fixer);
break;
case PatternKind.BANNED_NAME:
- engine = new NameEngine(
- config as Config<PatternKind.BANNED_NAME>,
- fixer as Fixer<MatchedNodeTypes[PatternKind.BANNED_NAME]>);
+ this.engine = new NameEngine(config, fixer);
break;
default:
throw new Error('Config type not recognized, or not implemented yet.');
}
- // clang-format on
this.ruleName = `conformance-pattern-${config.kind}`;
- this.engine = engine as PatternEngine<P>;
}
register(checker: Checker) {
diff --git a/internal/tsetse/util/fixer.ts b/internal/tsetse/util/fixer.ts
index 77fa9f8..2b76983 100644
--- a/internal/tsetse/util/fixer.ts
+++ b/internal/tsetse/util/fixer.ts
@@ -8,8 +8,8 @@
* ban-preset-pattern users). See also `buildReplacementFixer` for a simpler way
* of implementing a Fixer.
*/
-export interface Fixer<NodeType extends ts.Node = ts.Node> {
- getFixForFlaggedNode(node: NodeType): Fix|undefined;
+export interface Fixer {
+ getFixForFlaggedNode(node: ts.Node): Fix|undefined;
}
/**
diff --git a/internal/tsetse/util/pattern_config.ts b/internal/tsetse/util/pattern_config.ts
index e4dcc0d..21e9c25 100644
--- a/internal/tsetse/util/pattern_config.ts
+++ b/internal/tsetse/util/pattern_config.ts
@@ -1,11 +1,10 @@
-import * as ts from 'typescript';
/**
* The list of supported patterns useable in ConformancePatternRule. The
* patterns whose name match JSConformance patterns should behave similarly (see
* https://github.com/google/closure-compiler/wiki/JS-Conformance-Framework)
*/
-export enum PatternKind {
+export const enum PatternKind {
BANNED_NAME = 'banned-name',
BANNED_PROPERTY_WRITE = 'banned-property-write',
BANNED_PROPERTY_NON_CONSTANT_WRITE = 'banned-property-non-constant-write'
@@ -14,8 +13,8 @@
/**
* A config for ConformancePatternRule.
*/
-export interface Config<P extends PatternKind> {
- kind: P;
+export interface Config {
+ kind: PatternKind;
/**
* Values have a pattern-specific syntax.
@@ -79,10 +78,3 @@
/** Manually reviewed exceptions (supposedly okay). */
MANUALLY_REVIEWED
}
-
-/** Maps the type of nodes that each `PatternType` consumes. */
-export interface MatchedNodeTypes {
- [PatternKind.BANNED_PROPERTY_WRITE]: ts.BinaryExpression;
- [PatternKind.BANNED_PROPERTY_NON_CONSTANT_WRITE]: ts.BinaryExpression;
- [PatternKind.BANNED_NAME]: ts.Identifier;
-}
diff --git a/internal/tsetse/util/pattern_engines/name_engine.ts b/internal/tsetse/util/pattern_engines/name_engine.ts
index 332c6c6..3312619 100644
--- a/internal/tsetse/util/pattern_engines/name_engine.ts
+++ b/internal/tsetse/util/pattern_engines/name_engine.ts
@@ -4,14 +4,12 @@
import {debugLog, shouldExamineNode} from '../ast_tools';
import {Fixer} from '../fixer';
import {AbsoluteMatcher} from '../match_symbol';
-import {Config, MatchedNodeTypes, PatternKind} from '../pattern_config';
+import {Config} from '../pattern_config';
import {PatternEngine} from './pattern_engine';
-export class NameEngine extends PatternEngine<PatternKind.BANNED_NAME> {
+export class NameEngine extends PatternEngine {
private readonly matcher: AbsoluteMatcher;
- constructor(
- config: Config<PatternKind.BANNED_NAME>,
- fixer?: Fixer<MatchedNodeTypes[PatternKind.BANNED_NAME]>) {
+ constructor(config: Config, fixer?: Fixer) {
super(config, fixer);
// TODO: Support more than one single value here, or even build a
// multi-pattern engine. This would help for performance.
@@ -27,9 +25,8 @@
ts.SyntaxKind.Identifier, this.checkAndFilterResults.bind(this),
ErrorCode.CONFORMANCE_PATTERN);
}
-
- check(tc: ts.TypeChecker, n: MatchedNodeTypes[PatternKind.BANNED_NAME]):
- MatchedNodeTypes[PatternKind.BANNED_NAME]|undefined {
+
+ check(tc: ts.TypeChecker, n: ts.Node): ts.Node|undefined {
if (!shouldExamineNode(n) || n.getSourceFile().isDeclarationFile) {
return;
}
diff --git a/internal/tsetse/util/pattern_engines/pattern_engine.ts b/internal/tsetse/util/pattern_engines/pattern_engine.ts
index 6e01420..d6b60ce 100644
--- a/internal/tsetse/util/pattern_engines/pattern_engine.ts
+++ b/internal/tsetse/util/pattern_engines/pattern_engine.ts
@@ -2,20 +2,19 @@
import {Checker} from '../../checker';
import {Fix} from '../../failure';
import {Fixer} from '../../util/fixer';
-import {Config, MatchedNodeTypes, PatternKind} from '../../util/pattern_config';
+import {Config} from '../../util/pattern_config';
import {shouldExamineNode} from '../ast_tools';
/**
* A patternEngine is the logic that handles a specific PatternKind.
*/
-export abstract class PatternEngine<P extends PatternKind> {
+export abstract class PatternEngine {
private readonly whitelistedPrefixes: string[] = [];
private readonly whitelistedRegExps: RegExp[] = [];
private readonly whitelistMemoizer: Map<string, boolean> = new Map();
constructor(
- protected readonly config: Config<P>,
- protected readonly fixer?: Fixer<MatchedNodeTypes[P]>) {
+ protected readonly config: Config, protected readonly fixer?: Fixer) {
if (config.whitelistEntries) {
for (const e of config.whitelistEntries) {
if (e.prefix) {
@@ -42,15 +41,14 @@
* with what the engine looks for, i.e., AST matching. The whitelisting logic
* and fix generation are handled in `checkAndFilterResults`.
*/
- abstract check(tc: ts.TypeChecker, n: MatchedNodeTypes[P]):
- MatchedNodeTypes[P]|undefined;
+ abstract check(tc: ts.TypeChecker, n: ts.Node): ts.Node|undefined;
/**
* A wrapper for `check` that handles aspects of the analysis that are not
* engine-specific, and which defers to the subclass-specific logic
* afterwards.
*/
- checkAndFilterResults(c: Checker, n: MatchedNodeTypes[P]) {
+ checkAndFilterResults(c: Checker, n: ts.Node) {
if (!shouldExamineNode(n) || n.getSourceFile().isDeclarationFile) {
return;
}
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
index d9d3a97..08b6781 100644
--- a/internal/tsetse/util/pattern_engines/property_non_constant_write_engine.ts
+++ b/internal/tsetse/util/pattern_engines/property_non_constant_write_engine.ts
@@ -5,19 +5,15 @@
import {Fixer} from '../fixer';
import {isLiteral} from '../is_literal';
import {PropertyMatcher} from '../match_symbol';
-import {Config, MatchedNodeTypes, PatternKind} from '../pattern_config';
+import {Config} 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> {
+export class PropertyNonConstantWriteEngine extends PatternEngine {
private readonly matcher: PropertyMatcher;
- constructor(
- config: Config<BanKind>, fixer?: Fixer<MatchedNodeTypes[BanKind]>) {
+ constructor(config: Config, fixer?: Fixer) {
super(config, fixer);
// TODO: Support more than one single value here, or even build a
// multi-pattern engine. This would help for performance.
@@ -35,8 +31,7 @@
ErrorCode.CONFORMANCE_PATTERN);
}
- check(tc: ts.TypeChecker, n: MatchedNodeTypes[BanKind]):
- MatchedNodeTypes[BanKind]|undefined {
+ check(tc: ts.TypeChecker, n: ts.Node): ts.Node|undefined {
if (!isPropertyWriteExpression(n)) {
return;
}
diff --git a/internal/tsetse/util/pattern_engines/property_write_engine.ts b/internal/tsetse/util/pattern_engines/property_write_engine.ts
index 3b92f2d..640ff64 100644
--- a/internal/tsetse/util/pattern_engines/property_write_engine.ts
+++ b/internal/tsetse/util/pattern_engines/property_write_engine.ts
@@ -4,19 +4,15 @@
import {debugLog, isPropertyWriteExpression} from '../ast_tools';
import {Fixer} from '../fixer';
import {PropertyMatcher} from '../match_symbol';
-import {Config, MatchedNodeTypes, PatternKind} from '../pattern_config';
+import {Config} from '../pattern_config';
import {PatternEngine} from '../pattern_engines/pattern_engine';
-// Just for conciseness.
-type BanKind = PatternKind.BANNED_PROPERTY_WRITE;
-
/**
* The engine for BANNED_PROPERTY_WRITE.
*/
-export class PropertyWriteEngine extends PatternEngine<BanKind> {
+export class PropertyWriteEngine extends PatternEngine {
private readonly matcher: PropertyMatcher;
- constructor(
- config: Config<BanKind>, fixer?: Fixer<MatchedNodeTypes[BanKind]>) {
+ constructor(config: Config, fixer?: Fixer) {
super(config, fixer);
// TODO: Support more than one single value here, or even build a
// multi-pattern engine. This would help for performance.
@@ -33,8 +29,7 @@
ErrorCode.CONFORMANCE_PATTERN);
}
- check(tc: ts.TypeChecker, n: MatchedNodeTypes[BanKind]):
- MatchedNodeTypes[BanKind]|undefined {
+ check(tc: ts.TypeChecker, n: ts.Node): ts.Node|undefined {
if (!isPropertyWriteExpression(n)) {
return;
}
diff --git a/internal/tsetse/util/testing/test_support.ts b/internal/tsetse/util/testing/test_support.ts
index febf37a..c12e28a 100644
--- a/internal/tsetse/util/testing/test_support.ts
+++ b/internal/tsetse/util/testing/test_support.ts
@@ -85,7 +85,7 @@
toHaveNFailures(): jasmine.CustomMatcher {
return {
- compare: (actual: Failure[], expected: Number, config?: Config<any>) => {
+ compare: (actual: Failure[], expected: Number, config?: Config) => {
if (actual.length === expected) {
return {pass: true};
} else {
@@ -236,7 +236,7 @@
{fileName?: string, start?: number, end?: number, replacement?: string}
]): void;
- toHaveNFailures(expected: Number, config?: Config<any>): void;
+ toHaveNFailures(expected: Number, config?: Config): void;
}
}
}
