More tests for the ConformancePatterns, and fix substr-based confusion

Adding a bunch more tests around various more complex cases (shadowing, function parameters, and whitespace) revealed a bug where I mixed start/end and start/length patterns. So I'm taking the opportunity to switch to String.prototype.slice for the matcher, which is slightly less surprising in its behavior.

PiperOrigin-RevId: 250134904

internal/tsetse/tests/ban_conformance_pattern/property_write_test.ts

diff --git a/internal/tsetse/tests/ban_conformance_pattern/property_write_test.ts b/internal/tsetse/tests/ban_conformance_pattern/property_write_test.ts
index 997577e..42290f2 100644
--- a/internal/tsetse/tests/ban_conformance_pattern/property_write_test.ts
+++ b/internal/tsetse/tests/ban_conformance_pattern/property_write_test.ts
@@ -3,19 +3,63 @@
 import {compileAndCheck, customMatchers} from '../../util/testing/test_support';
 
 describe('BANNED_PROPERTY_WRITE', () => {
+  const rule = new ConformancePatternRule({
+    errorMessage: 'do not cite',
+    kind: PatternKind.BANNED_PROPERTY_WRITE,
+    values: ['HTMLQuoteElement.prototype.cite']
+  });
+
   it('matches simple examples', () => {
-    const source = `const q = document.createElement('q');\n` +
-        `q.cite = 'some example string';\n`;
-    const rule = new ConformancePatternRule({
-      errorMessage: 'do not cite',
-      kind: PatternKind.BANNED_PROPERTY_WRITE,
-      values: ['HTMLQuoteElement.prototype.cite']
-    });
+    const source = [
+      `const q = document.createElement('q');`,
+      `q.cite = 'some example string';`,
+    ].join('\n');
     const results = compileAndCheck(rule, source);
 
     expect(results.length).toBe(1);
-    expect(results[0])
-        .toBeFailureMatching({start: 39, end: 69, errorMessage: 'do not cite'});
+    expect(results[0]).toBeFailureMatching({
+      matchedCode: `q.cite = 'some example string'`,
+      errorMessage: 'do not cite'
+    });
+  });
+
+  it('matches precisely, even with whitespace or comments', () => {
+    const source = [
+      `const q = document.createElement('q');`,
+      `    q.cite = 'exampleA';`,
+      `q.cite = 'exampleB'    ;`,
+      `/* test1 */ q.cite = /* test2 */ 'exampleC' /* test3 */;`,
+    ].join('\n');
+    const results = compileAndCheck(rule, source);
+
+    expect(results.length).toBe(3);
+    expect(results[0]).toBeFailureMatching({
+      matchedCode: `q.cite = 'exampleA'`,
+      errorMessage: 'do not cite'
+    });
+    expect(results[1]).toBeFailureMatching({
+      matchedCode: `q.cite = 'exampleB'`,
+      errorMessage: 'do not cite'
+    });
+    expect(results[2]).toBeFailureMatching({
+      matchedCode: `q.cite = /* test2 */ 'exampleC'`,
+      errorMessage: 'do not cite'
+    });
+  })
+
+  it('understands function prototypes', () => {
+    const source = [
+      `function foo(q:HTMLQuoteElement) {`,
+      `    q.cite = 'some example string';`,
+      `}`,
+    ].join('\n');
+    const results = compileAndCheck(rule, source);
+
+    expect(results.length).toBe(1);
+    expect(results[0]).toBeFailureMatching({
+      matchedCode: `q.cite = 'some example string'`,
+      errorMessage: 'do not cite'
+    });
   });
 
   it('understands imported symbols', () => {
@@ -23,16 +67,11 @@
       `const q = document.createElement('q'); export {q};`,
       `import {q} from './file_0'; q.cite = window.name;`
     ];
-    const rule = new ConformancePatternRule({
-      errorMessage: 'do not cite',
-      kind: PatternKind.BANNED_PROPERTY_WRITE,
-      values: ['HTMLQuoteElement.prototype.cite']
-    });
     const results = compileAndCheck(rule, ...sources);
 
     expect(results.length).toBe(1);
     expect(results[0]).toBeFailureMatching({
-      matchedCode: 'q.cite = window.name;',
+      matchedCode: 'q.cite = window.name',
       fileName: 'file_1.ts',
       errorMessage: 'do not cite',
     });
@@ -41,16 +80,16 @@
 
   describe('with inheritance', () => {
     const source = [
-      `class Parent {x:number}`,
+      `class Parent { x: number }`,
       `class Child extends Parent {}`,
-      `const c:Child = new Child();`,
+      `const c: Child = new Child();`,
       `c.x = 1;`,
     ].join('\n');
 
     // Both of these should have the same results: in `c.x`, `x` matches,
     // and `c` is both a Parent and a Child.
     const expectedFailure = {
-      matchedCode: 'c.x = 1;',
+      matchedCode: 'c.x = 1',
       errorMessage: 'found write to x',
     };
 
@@ -76,6 +115,23 @@
       expect(r[0]).toBeFailureMatching(expectedFailure);
     });
   });
+
+  describe('with shadowing', () => {
+    it('does not over-match', () => {
+      const source = [
+        `const q = document.createElement('q');`,
+        `const f1 = (q: {cite: string}) => { q.cite = 'example 1'; };`,
+      ].join('\n');
+      const rule = new ConformancePatternRule({
+        errorMessage: 'do not cite',
+        kind: PatternKind.BANNED_PROPERTY_WRITE,
+        values: ['HTMLQuoteElement.prototype.cite']
+      });
+      const results = compileAndCheck(rule, source);
+
+      expect(results.length).toBe(0);
+    });
+  });
 });
 
 beforeEach(() => {

internal/tsetse/util/pattern_engines/property_write_engine.ts

diff --git a/internal/tsetse/util/pattern_engines/property_write_engine.ts b/internal/tsetse/util/pattern_engines/property_write_engine.ts
index 7af7ccf..ea90e60 100644
--- a/internal/tsetse/util/pattern_engines/property_write_engine.ts
+++ b/internal/tsetse/util/pattern_engines/property_write_engine.ts
@@ -38,10 +38,12 @@
         !isPropertyWriteExpression(n)) {
       return;
     }
-    debugLog(`inspecting ${n.getFullText().trim()}`);
+    debugLog(`inspecting ${n.getText().trim()}`);
     if (this.matcher.matches(n.left, c.typeChecker)) {
       const fix: Fix|undefined =
           this.fixer ? this.fixer.getFixForFlaggedNode(n) : undefined;
+      debugLog(`Match. Reporting failure (boundaries: ${n.getStart()}, ${
+          n.getEnd()}] on node [${n.getText()}]`);
       c.addFailureAtNode(n, this.config.errorMessage, fix);
     }
   }

internal/tsetse/util/testing/test_support.ts

diff --git a/internal/tsetse/util/testing/test_support.ts b/internal/tsetse/util/testing/test_support.ts
index f79192c..6177710 100644
--- a/internal/tsetse/util/testing/test_support.ts
+++ b/internal/tsetse/util/testing/test_support.ts
@@ -79,7 +79,7 @@
         const actualDiagnostic = actualFailure.toDiagnostic();
         let regrets = '';
         if (exp === undefined) {
-          regrets += 'The rule requires two arguments. ';
+          regrets += 'The matcher requires two arguments. ';
         }
         if (exp.fileName) {
           if (!actualDiagnostic.file) {
@@ -100,12 +100,17 @@
           if (!actualDiagnostic.file) {
             regrets += `Expected diagnostic to have a source file, but it had ${
                 actualDiagnostic.file}. `;
+          } else if (!actualDiagnostic.start) {
+            // I don't know how this could happen, but typings say so.
+            regrets += `Expected diagnostic to have a starting position. `;
           } else {
-            const foundMatchedCode = actualDiagnostic.file.getFullText().substr(
+            const foundMatchedCode = actualDiagnostic.file.getFullText().slice(
                 Number(actualDiagnostic.start), actualDiagnostic.end);
             if (foundMatchedCode != exp.matchedCode) {
               regrets += `Expected diagnostic to match ${
-                  exp.matchedCode}, but was ${foundMatchedCode}`;
+                  exp.matchedCode}, but was ${foundMatchedCode} (from ${
+                  Number(
+                      actualDiagnostic.start)} to ${actualDiagnostic.end}). `;
             }
           }
         }