Repository: rokucommunity/brighterscript
Pull request #865 (2023-08-01 14:04:48)

Fix tab issue when printing diagnostics
Normalize tab and spaces when printing diagnostics to the console. 

Example:
for this line:
`\t\tdescription = description.left(description.len() - 2) + "]"`

Turns this:
![image](https://github.com/rokucommunity/brighterscript/assets/2544493/243ce6f2-c11c-4f61-a5ad-918a3e546b2c)

into this:
![image](https://github.com/rokucommunity/brighterscript/assets/2544493/4e3a290e-6988-4173-8043-a625e9a83f2c)



Patch:
diff --git a/src/diagnosticUtils.spec.ts b/src/diagnosticUtils.spec.ts
index e8360a37..05a075c4 100644
--- a/src/diagnosticUtils.spec.ts
+++ b/src/diagnosticUtils.spec.ts
@@ -4,13 +4,23 @@ import * as diagnosticUtils from './diagnosticUtils';
 import { Range, DiagnosticSeverity } from 'vscode-languageserver';
 import { util } from './util';
 import chalk from 'chalk';
+import { createSandbox } from 'sinon';
+import undent from 'undent';
+import type { BsDiagnostic } from './interfaces';
+import { stripConsoleColors } from './testHelpers.spec';
+const sinon = createSandbox();
 
 describe('diagnosticUtils', () => {
     let options: ReturnType<typeof diagnosticUtils.getPrintDiagnosticOptions>;
     beforeEach(() => {
+        sinon.restore();
         options = diagnosticUtils.getPrintDiagnosticOptions({});
     });
 
+    afterEach(() => {
+        sinon.restore();
+    });
+
     describe('printDiagnostic', () => {
         it('does not crash when range is undefined', () => {
             //print a diagnostic that doesn't have a range...it should not explode
@@ -25,10 +35,73 @@ describe('diagnosticUtils', () => {
             //print a diagnostic that doesn't have a range...it should not explode
             diagnosticUtils.printDiagnostic(options, DiagnosticSeverity.Error, undefined, [], {
                 message: 'Bad thing happened',
-                range: Range.create(0, 0, 2, 2), //important...this needs to be null for the test to pass,
+                range: Range.create(0, 0, 2, 2),
                 code: 1234
             } as any);
         });
+
+        function testPrintDiagnostic(diagnostic: BsDiagnostic, code: string, expected: string) {
+            let logOutput = '';
+            sinon.stub(console, 'log').callsFake((...args: any[]) => {
+                if (logOutput.length > 0) {
+                    logOutput += '\n';
+                }
+                logOutput += stripConsoleColors(args.join(' '));
+            });
+            //print a diagnostic that doesn't have a range...it should not explode
+            diagnosticUtils.printDiagnostic(options, DiagnosticSeverity.Error, undefined, code.split(/\r?\n/g), diagnostic);
+
+            //remove leading and trailing newlines
+            logOutput = logOutput.replace(/^[\r\n]*/g, '').replace(/[\r\n]*$/g, '');
+            expected = undent(logOutput).replace(/^[\r\n]*/g, '').replace(/[\r\n]*$/g, '');
+
+            expect(logOutput).to.eql(expected);
+        }
+
+        it('handles mixed tabs and spaces', () => {
+            testPrintDiagnostic(
+                {
+                    message: 'Bad thing happened',
+                    range: Range.create(0, 5, 0, 18),
+                    code: 1234
+                } as any,
+                `\t  \t print "hello"`,
+                `
+                <unknown file>:1:6 - error BS1234: Bad thing happened
+                 1             print "hello"
+                 _             ~~~~~~~~~~~~~
+            `);
+        });
+
+        it('handles only tabs', () => {
+            testPrintDiagnostic(
+                {
+                    message: 'Bad thing happened',
+                    range: Range.create(0, 5, 0, 18),
+                    code: 1234
+                } as any,
+                `\tprint "hello"`,
+                `
+                <unknown file>:1:6 - error BS1234: Bad thing happened
+                 1      print "hello"
+                 _      ~~~~~~~~~~~~~
+            `);
+        });
+
+        it('handles only spaces', () => {
+            testPrintDiagnostic(
+                {
+                    message: 'Bad thing happened',
+                    range: Range.create(0, 5, 0, 18),
+                    code: 1234
+                } as any,
+                `   print "hello"`,
+                `
+                <unknown file>:1:6 - error BS1234: Bad thing happened
+                 1     print "hello"
+                 _     ~~~~~~~~~~~~~
+            `);
+        });
     });
 
     describe('getPrintDiagnosticOptions', () => {
@@ -92,76 +165,75 @@ describe('diagnosticUtils', () => {
 
     describe('getDiagnosticSquiggly', () => {
         it('works for normal cases', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 0, 0, 4)
-            }, 'asdf')).to.equal('~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('asdf', 0, 4)
+            ).to.equal('~~~~');
         });
 
         it('highlights whole line if no range', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-            }, ' asdf ')).to.equal('~~~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText(' asdf ', undefined, undefined)
+            ).to.equal('~~~~~~');
         });
 
         it('returns empty string when no line is found', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 0, 0, 10)
-            }, '')).to.equal('');
+            expect(diagnosticUtils.getDiagnosticSquigglyText('', 0, 10)).to.equal('');
 
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 0, 0, 10)
-            }, undefined)).to.equal('');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText(undefined, 0, 10)
+            ).to.equal('');
         });
 
         it('supports diagnostic not at start of line', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 2, 0, 6)
-            }, '  asdf')).to.equal('  ~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('  asdf', 2, 6)
+            ).to.equal('  ~~~~');
         });
 
         it('supports diagnostic that does not finish at end of line', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 0, 0, 4)
-            }, 'asdf  ')).to.equal('~~~~  ');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('asdf  ', 0, 4)
+            ).to.equal('~~~~  ');
         });
 
         it('supports diagnostic with space on both sides', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 2, 0, 6)
-            }, '  asdf  ')).to.equal('  ~~~~  ');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('  asdf  ', 2, 6)
+            ).to.equal('  ~~~~  ');
         });
 
         it('handles diagnostic that starts and stops on the same position', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 2, 0, 2)
-            }, 'abcde')).to.equal('~~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('abcde', 2, 2)
+            ).to.equal('~~~~~');
         });
 
         it('handles single-character diagnostic', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 2, 0, 3)
-            }, 'abcde')).to.equal('  ~  ');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('abcde', 2, 3)
+            ).to.equal('  ~  ');
         });
 
         it('handles diagnostics that are longer than the line', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 0, 0, 10)
-            }, 'abcde')).to.equal('~~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('abcde', 0, 10)
+            ).to.equal('~~~~~');
 
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(0, 2, 0, 10)
-            }, 'abcde')).to.equal('  ~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('abcde', 2, 10)
+            ).to.equal('  ~~~');
         });
 
         it('handles Number.MAX_VALUE for end character', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: util.createRange(0, 0, 0, Number.MAX_VALUE)
-            }, 'abcde')).to.equal('~~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('abcde', 0, Number.MAX_VALUE)
+            ).to.equal('~~~~~');
         });
 
         it.skip('handles edge cases', () => {
-            expect(diagnosticUtils.getDiagnosticSquigglyText(<any>{
-                range: Range.create(5, 16, 5, 18)
-            }, 'end functionasdf')).to.equal('            ~~~~');
+            expect(
+                diagnosticUtils.getDiagnosticSquigglyText('end functionasdf', 16, 18)
+            ).to.equal('            ~~~~');
         });
     });
 
diff --git a/src/diagnosticUtils.ts b/src/diagnosticUtils.ts
index 4a9db383..af621463 100644
--- a/src/diagnosticUtils.ts
+++ b/src/diagnosticUtils.ts
@@ -101,9 +101,20 @@ export function getDiagnosticLine(diagnostic: BsDiagnostic, diagnosticLine: stri
     if (diagnostic.range && diagnosticLine) {
         const lineNumberText = chalk.bgWhite(' ' + chalk.black((diagnostic.range.start.line + 1).toString()) + ' ') + ' ';
         const blankLineNumberText = chalk.bgWhite(' ' + chalk.white('_'.repeat((diagnostic.range.start.line + 1).toString().length)) + ' ') + ' ';
-        const squigglyText = getDiagnosticSquigglyText(diagnostic, diagnosticLine);
+
+        //remove tabs in favor of spaces to make diagnostic printing more consistent
+        let leadingText = diagnosticLine.slice(0, diagnostic.range.start.character);
+        let leadingTextNormalized = leadingText.replace(/\t/g, '    ');
+        let actualText = diagnosticLine.slice(diagnostic.range.start.character, diagnostic.range.end.character);
+        let actualTextNormalized = actualText.replace(/\t/g, '    ');
+        let startIndex = leadingTextNormalized.length;
+        let endIndex = leadingTextNormalized.length + actualTextNormalized.length;
+
+        let diagnosticLineNormalized = diagnosticLine.replace(/\t/g, '    ');
+
+        const squigglyText = getDiagnosticSquigglyText(diagnosticLineNormalized, startIndex, endIndex);
         result +=
-            lineNumberText + diagnosticLine + '\n' +
+            lineNumberText + diagnosticLineNormalized + '\n' +
             blankLineNumberText + colorFunction(squigglyText);
     }
     return result;
@@ -112,36 +123,36 @@ export function getDiagnosticLine(diagnostic: BsDiagnostic, diagnosticLine: stri
 /**
  * Given a diagnostic, compute the range for the squiggly
  */
-export function getDiagnosticSquigglyText(diagnostic: BsDiagnostic, line: string) {
+export function getDiagnosticSquigglyText(line: string, startCharacter: number, endCharacter: number) {
     let squiggle: string;
     //fill the entire line
     if (
         //there is no range
-        !diagnostic.range ||
+        typeof startCharacter !== 'number' || typeof endCharacter !== 'number' ||
         //there is no line
         !line ||
         //both positions point to same location
-        diagnostic.range.start.character === diagnostic.range.end.character ||
+        startCharacter === endCharacter ||
         //the diagnostic starts after the end of the line
-        diagnostic.range.start.character >= line.length
+        startCharacter >= line.length
     ) {
         squiggle = ''.padStart(line?.length ?? 0, '~');
     } else {
 
-        let endIndex = Math.max(diagnostic.range?.end.character, line.length);
+        let endIndex = Math.max(endCharacter, line.length);
         endIndex = endIndex > 0 ? endIndex : 0;
         if (line?.length < endIndex) {
             endIndex = line.length;
         }
 
-        let leadingWhitespaceLength = diagnostic.range.start.character;
+        let leadingWhitespaceLength = startCharacter;
         let squiggleLength: number;
-        if (diagnostic.range.end.character === Number.MAX_VALUE) {
+        if (endCharacter === Number.MAX_VALUE) {
             squiggleLength = line.length - leadingWhitespaceLength;
         } else {
-            squiggleLength = diagnostic.range.end.character - diagnostic.range.start.character;
+            squiggleLength = endCharacter - startCharacter;
         }
-        let trailingWhitespaceLength = endIndex - diagnostic.range.end.character;
+        let trailingWhitespaceLength = endIndex - endCharacter;
 
         //opening whitespace
         squiggle =


Test patch:
diff --git a/src/testHelpers.spec.ts b/src/testHelpers.spec.ts
index f00ff443..a865fd74 100644
--- a/src/testHelpers.spec.ts
+++ b/src/testHelpers.spec.ts
@@ -362,3 +362,12 @@ export function mapToObject<T>(map: Map<any, T>) {
     }
     return result;
 }
+
+export function stripConsoleColors(inputString) {
+    // Regular expression to match ANSI escape codes for colors
+    // eslint-disable-next-line no-control-regex
+    const colorPattern = /\u001b\[(?:\d*;){0,5}\d*m/g;
+
+    // Remove all occurrences of ANSI escape codes
+    return inputString.replace(colorPattern, '');
+}