NEW @W-19374413@ Fixed test

Author: jfeingold35

.gitattributes

@@ -4,4 +4,7 @@
 # See https://help.github.com/articles/dealing-with-line-endings/
 *.goldfile.*    text eol=lf
 
+# Some of our input files must be forced to use a certain EOL signifier no matter what the OS wants.
+# So we forcefully set any file that includes lf or crlf in its file extension to use the corresponding EOL.
 *.lf.* text eol=lf
+*.crlf.* text eol=crlf

packages/code-analyzer-regex-engine/src/engine.ts

@@ -11,7 +11,6 @@ import {
 import path from "node:path";
 import fs from "node:fs";
 import * as fsp from 'node:fs/promises';
-import os from "node:os";
 import {RegexRule, RegexRules} from "./config";
 import {isBinaryFile} from "isbinaryfile";
 import {convertToRegex, PromiseExecutionLimiter} from "./utils";
@@ -137,7 +136,8 @@ export class RegexEngine extends Engine {
     private async scanFileContents(fileName: string, fileContents: string, ruleName: string): Promise<Violation[]> {
         const violations: Violation[] = [];
         const regex: RegExp = this.getRegExpFor(ruleName);
-        const newlineIndexes: number[] = getNewlineIndices(fileContents);
+        const contextuallyDerivedEol: string = contextuallyDeriveEolString(fileContents);
+        const newlineIndexes: number[] = getNewlineIndices(fileContents, contextuallyDerivedEol);
 
         for (const match of fileContents.matchAll(regex)) {
             let startIndex: number = match.index;
@@ -151,9 +151,9 @@ export class RegexEngine extends Engine {
             }
 
             const startLine: number = getLineNumber(startIndex, newlineIndexes);
-            const startColumn: number = getColumnNumber(startIndex, newlineIndexes);
+            const startColumn: number = getColumnNumber(startIndex, newlineIndexes, contextuallyDerivedEol);
             const endLine: number = getLineNumber(startIndex + matchLength, newlineIndexes);
-            const endColumn: number = getColumnNumber(startIndex + matchLength, newlineIndexes);
+            const endColumn: number = getColumnNumber(startIndex + matchLength, newlineIndexes, contextuallyDerivedEol);
             const codeLocation: CodeLocation = {
                 file: fileName,
                 startLine: startLine,
@@ -180,13 +180,13 @@ export class RegexEngine extends Engine {
 }
 
 
-function getColumnNumber(charIndex: number, newlineIndexes: number[]): number {
+function getColumnNumber(charIndex: number, newlineIndexes: number[], contextuallyDerivedEol: string): number {
     const idxOfNextNewline = newlineIndexes.findIndex(el => el >= charIndex);
     const idxOfCurrentLine = idxOfNextNewline === -1 ? newlineIndexes.length - 1: idxOfNextNewline - 1;
     if (idxOfCurrentLine === 0){
         return charIndex + 1;
     } else {
-        const eolOffset = os.EOL.length - 1;
+        const eolOffset = contextuallyDerivedEol.length - 1;
         return charIndex - newlineIndexes.at(idxOfCurrentLine)! - eolOffset;
     }
 }
@@ -196,8 +196,8 @@ function getLineNumber(charIndex: number, newlineIndexes: number[]): number{
     return idxOfNextNewline === -1 ? newlineIndexes.length : idxOfNextNewline;
 }
 
-function getNewlineIndices(fileContents: string): number[] {
-    const newlineRegex: RegExp = new RegExp(os.EOL, "g");
+function getNewlineIndices(fileContents: string, contextuallyDerivedEol: string): number[] {
+    const newlineRegex: RegExp = new RegExp(contextuallyDerivedEol, "g");
     const matches = fileContents.matchAll(newlineRegex);
     const newlineIndexes = [-1];
 
@@ -207,6 +207,15 @@ function getNewlineIndices(fileContents: string): number[] {
     return newlineIndexes;
 }
 
+/**
+ * We can't assume that the file's EOL indicator matches the OS's, because of edge cases such as a file created on Unix
+ * but scanned on Windows. So instead of consulting the OS for the EOL indicator, we look at the file and find which one it uses.
+ * @param contents
+ */
+function contextuallyDeriveEolString(contents: string): string {
+    return contents.indexOf('\r\n') !== -1 ? '\r\n' : '\n';
+}
+
 async function isTextFile(fileName: string): Promise<boolean> {
     const ext: string = path.extname(fileName).toLowerCase();
     return TEXT_BASED_FILE_EXTS.has(ext) || !(await isBinaryFile(fileName));

packages/code-analyzer-regex-engine/test/engine.test.ts

@@ -39,7 +39,7 @@ const SAMPLE_CUSTOM_RULES: RegexRules = {
     NoTalkingAboutFightClub: {
         regex: '/fight club/gi',
         description: "The first rule of Fight Club is do not talk about Fight Club",
-        file_extensions: [".abc-def.xyz", ".lf.cls"],
+        file_extensions: [".abc-def.xyz", ".lf.cls", ".crlf.cls"],
         violation_message: "The second rule of Fight Club is DO NOT. TALK. ABOUT FIGHT CLUB.",
         severity: DEFAULT_SEVERITY_LEVEL,
         tags: ['PopCulture']
@@ -783,9 +783,9 @@ describe('Tests for runRules', () => {
         }
     });
 
-    it('When workspace contains files that use \\n instead of \\r\\n, violations are right regardless of OS', async () => {
+    it('When files indicate EOL differently than OS, violations are still correct', async () => {
         const runOptions: RunOptions = createRunOptions(
-            new Workspace('id', [path.resolve(__dirname, "test-data", "workspaceWithLfNewlines")]));
+            new Workspace('id', [path.resolve(__dirname, "test-data", "workspaceWithOddNewlines")]));
         const runResults: EngineRunResults = await engine.runRules(['NoTalkingAboutFightClub'], runOptions);
 
         const expectedViolations: Violation[] = [
@@ -795,7 +795,21 @@ describe('Tests for runRules', () => {
                 primaryLocationIndex: 0,
                 codeLocations: [
                     {
-                        file: path.resolve(__dirname, "test-data", "workspaceWithLfNewlines", "testClass.lf.cls"),
+                        file: path.resolve(__dirname, "test-data", "workspaceWithOddNewlines", "testClass.lf.cls"),
+                        startLine: 3,
+                        startColumn: 87,
+                        endLine: 3,
+                        endColumn: 97
+                    }
+                ]
+            },
+            {
+                ruleName: "NoTalkingAboutFightClub",
+                message: "The second rule of Fight Club is DO NOT. TALK. ABOUT FIGHT CLUB.",
+                primaryLocationIndex: 0,
+                codeLocations: [
+                    {
+                        file: path.resolve(__dirname, "test-data", "workspaceWithOddNewlines", "testClass.crlf.cls"),
                         startLine: 3,
                         startColumn: 87,
                         endLine: 3,

packages/code-analyzer-regex-engine/test/test-data/workspaceWithOddNewlines/testClass.crlf.cls

@@ -0,0 +1,6 @@
+public class TestClass{
+    public static boolean doTheThing() {
+        List<String> strings = new List<String>{'This is fine', 'This is also fine', 'fight club', 'OH NO, that last string was bad!'};
+        return false;
+    }
+}