Fix some sourcemap transpile issues (#249)

very thoughtful of you to change the colors: I get (no pun intended) blind to these sometimes. Thanks.
rokucommunity · Jan 6, 2024 · 9eea489 · 9eea489
1 parent 8e08407
commit 9eea489
Show file tree

Hide file tree

Showing 7 changed files with 145 additions and 49 deletions.
diff --git a/bsc-plugin/package.json b/bsc-plugin/package.json
@@ -27,19 +27,19 @@
         "@types/node": "^14.18.41",
         "@typescript-eslint/eslint-plugin": "^5.27.0",
         "@typescript-eslint/parser": "^5.27.0",
-        "cz-conventional-changelog": "^3.3.0",
-        "brighterscript": "^0.61.3",
+        "brighterscript": "^0.65.15",
         "chai": "^4.2.0",
         "chai-subset": "^1.6.0",
         "coveralls": "^3.0.0",
+        "cz-conventional-changelog": "^3.3.0",
         "eslint": "^8.16.0",
         "eslint-plugin-no-only-tests": "^2.4.0",
         "fs-extra": "^10.1.0",
         "minimatch": "^3.0.4",
         "mocha": "^9.1.3",
         "nyc": "^15.1.0",
-        "source-map-support": "^0.5.13",
         "release-it": "^15.10.3",
+        "source-map-support": "^0.5.13",
         "trim-whitespace": "^1.3.3",
         "ts-node": "^9.0.0",
         "typescript": "^4.9.5"

diff --git a/bsc-plugin/src/lib/rooibos/MockUtil.ts b/bsc-plugin/src/lib/rooibos/MockUtil.ts
@@ -9,11 +9,10 @@ import { Range } from 'vscode-languageserver-types';
 import type { FileFactory } from './FileFactory';
 import undent from 'undent';
 import type { RooibosSession } from './RooibosSession';
-import { BrsTranspileState } from 'brighterscript/dist/parser/BrsTranspileState';
 import { diagnosticErrorProcessingFile } from '../utils/Diagnostics';
 import type { TestCase } from './TestCase';
 import type { TestSuite } from './TestSuite';
-import { getAllDottedGetParts, getRootObjectFromDottedGet, getStringPathFromDottedGet, overrideAstTranspile } from './Utils';
+import { getAllDottedGetParts } from './Utils';
 
 export class MockUtil {
 
@@ -188,4 +187,3 @@ export class MockUtil {
     }
 
 }
-
diff --git a/bsc-plugin/src/lib/rooibos/TestGroup.ts b/bsc-plugin/src/lib/rooibos/TestGroup.ts
@@ -1,16 +1,13 @@
-import type { AstEditor, CallExpression, DottedGetExpression, Expression } from 'brighterscript';
-import { isCallExpression, isCallfuncExpression, isIndexedGetExpression, ArrayLiteralExpression, createInvalidLiteral, createStringLiteral, createToken, isDottedGetExpression, TokenKind, isLiteralExpression, isVariableExpression, isFunctionExpression } from 'brighterscript';
+import type { AstEditor, CallExpression, DottedGetExpression } from 'brighterscript';
+import { ArrayLiteralExpression, createInvalidLiteral, createStringLiteral, createToken, isDottedGetExpression, TokenKind, isFunctionExpression, Parser } from 'brighterscript';
 import * as brighterscript from 'brighterscript';
 import { BrsTranspileState } from 'brighterscript/dist/parser/BrsTranspileState';
-import { TranspileState } from 'brighterscript/dist/parser/TranspileState';
 import { diagnosticErrorProcessingFile } from '../utils/Diagnostics';
 import type { RooibosAnnotation } from './Annotation';
-import { RawCodeStatement } from './RawCodeStatement';
 import type { TestCase } from './TestCase';
 import type { TestSuite } from './TestSuite';
 import { TestBlock } from './TestSuite';
-import { getAllDottedGetParts, getRootObjectFromDottedGet, getStringPathFromDottedGet, overrideAstTranspile, sanitizeBsJsonString } from './Utils';
-import undent from 'undent';
+import { getAllDottedGetParts, getRootObjectFromDottedGet, getStringPathFromDottedGet, sanitizeBsJsonString } from './Utils';
 import type { NamespaceContainer } from './RooibosSession';
 
 export class TestGroup extends TestBlock {
@@ -59,7 +56,7 @@ export class TestGroup extends TestBlock {
         try {
             let func = this.testSuite.classStatement.methods.find((m) => m.name.text.toLowerCase() === testCase.funcName.toLowerCase());
             func.walk(brighterscript.createVisitor({
-                ExpressionStatement: (expressionStatement, parent, owner) => {
+                ExpressionStatement: (expressionStatement, parent, owner, key) => {
                     let callExpression = expressionStatement.expression as CallExpression;
                     if (brighterscript.isCallExpression(callExpression) && brighterscript.isDottedGetExpression(callExpression.callee)) {
                         let dge = callExpression.callee;
@@ -75,12 +72,15 @@ export class TestGroup extends TestBlock {
                                 if (dge.name.text === 'expectCalled' || dge.name.text === 'expectNotCalled') {
                                     this.modifyModernRooibosExpectCallExpression(callExpression, editor, namespaceLookup);
                                 }
-                                //TODO change this to editor.setProperty(parentObj, parentKey, new SourceNode()) once bsc supports it
-                                overrideAstTranspile(editor, expressionStatement, '\n' + undent`
-                                    m.currentAssertLineNumber = ${callExpression.range.start.line}
-                                    ${callExpression.transpile(transpileState).join('')}
-                                    ${noEarlyExit ? '' : `if m.currentResult?.isFail = true then m.done() : return ${isSub ? '' : 'invalid'}`}
-                                ` + '\n');
+                                if (dge.name.text === 'expectCalled' || dge.name.text === 'expectNotCalled') {
+                                    this.modifyModernRooibosExpectCallExpression(callExpression, editor, namespaceLookup);
+                                }
+                                const trailingLine = Parser.parse(`${noEarlyExit ? '' : `if m.currentResult?.isFail = true then m.done() : return ${isSub ? '' : 'invalid'}`}`).ast.statements[0];
+
+                                editor.arraySplice(owner, key + 1, 0, trailingLine);
+
+                                const leadingLine = Parser.parse(`m.currentAssertLineNumber = ${callExpression.range.start.line}`).ast.statements[0];
+                                editor.arraySplice(owner, key, 0, leadingLine);
                             }
                         }
                     }

diff --git a/bsc-plugin/src/lib/rooibos/Utils.ts b/bsc-plugin/src/lib/rooibos/Utils.ts
@@ -1,23 +1,6 @@
-import type { BrsFile, ClassStatement, Expression, FunctionStatement, Statement, AnnotationExpression, AstEditor } from 'brighterscript';
+import type { BrsFile, ClassStatement, Expression, FunctionStatement, AnnotationExpression, AstEditor } from 'brighterscript';
 import * as brighterscript from 'brighterscript';
 import { diagnosticCorruptTestProduced } from '../utils/Diagnostics';
-import { SourceNode } from 'source-map';
-
-export function overrideAstTranspile(editor: AstEditor, node: Expression | Statement, value: string) {
-    editor.setProperty(node, 'transpile', function transpile(this: Expression | Statement, state) {
-        //indent every line with the current transpile indent level (except the first line, because that's pre-indented by bsc)
-        let source = value.replace(/\r?\n/g, (match, newline) => {
-            return state.newline + state.indent();
-        });
-
-        return [new SourceNode(
-            this.range.start.line + 1,
-            this.range.start.character,
-            state.srcPath,
-            source
-        )];
-    });
-}
 
 export function addOverriddenMethod(file: BrsFile, annotation: AnnotationExpression, target: ClassStatement, name: string, source: string, editor: AstEditor): boolean {
     let functionSource = `

diff --git a/bsc-plugin/src/plugin.spec.ts b/bsc-plugin/src/plugin.spec.ts
@@ -1,11 +1,12 @@
 import type { BrsFile, CallExpression, ClassMethodStatement, ClassStatement, ExpressionStatement, FunctionStatement } from 'brighterscript';
-import { CallfuncExpression, DiagnosticSeverity, DottedGetExpression, Program, ProgramBuilder, util, standardizePath as s, PrintStatement } from 'brighterscript';
+import { CallfuncExpression, DiagnosticSeverity, DottedGetExpression, Position, Program, ProgramBuilder, util, standardizePath as s, PrintStatement } from 'brighterscript';
 import { expect } from 'chai';
 import { RooibosPlugin } from './plugin';
 import * as fsExtra from 'fs-extra';
 import * as path from 'path';
 import * as trim from 'trim-whitespace';
 import undent from 'undent';
+import { SourceMapConsumer } from 'source-map';
 let tmpPath = s`${process.cwd()}/tmp`;
 let _rootDir = s`${tmpPath}/rootDir`;
 let _stagingFolderPath = s`${tmpPath}/staging`;
@@ -363,6 +364,61 @@ describe('RooibosPlugin', () => {
             expect(plugin.session.sessionInfo.testSuitesToRun).to.be.empty;
         });
 
+        it('sanity check for sourcemaps', async () => {
+            await testSourcemapLocations(`
+                sub main()
+                    print "hello"
+                end sub
+            `, `
+                sub main()
+                    Rooibos_init("RooibosScene")
+                    print "hello"
+                end sub'//# sourceMappingURL=./test.spec.bs.map
+            `, [
+                // print "h|ello" => print |"hello"
+                { dest: [2, 12], src: [2, 26] }
+            ]);
+        });
+
+        it('produces proper sourcemaps', async () => {
+            await testSourcemapLocations(`
+                @suite
+                class ATest
+                    @describe("groupA")
+                    @describe("groupB")
+                    @it("is test1")
+                    function Test_3()
+                        number = 123
+                        m.assertEqual("123", \`alpha-\${number}-beta\`)
+                        m.assertEqual(123, 123)
+                    end function
+                end class
+            `, `
+                function __ATest_builder()
+                    instance = {}
+                    instance.new = sub()
+                    end sub
+                    instance.groupB_is_test1 = function()
+                        number = 123
+                        m.assertEqual("123", ("alpha-" + bslib_toString(number) + "-beta"))
+                        m.assertEqual(123, 123)
+                    end function
+                    return instance
+                end function
+                function ATest()
+                    instance = __ATest_builder()
+                    instance.new()
+                    return instance
+                end function'//# sourceMappingURL=./test.spec.bs.map
+            `, [
+                // m.assert|Equal("123", ("alpha-" + bslib_toString(number) + "-beta"))    =>    m.|assertEqual("123", `alpha-${number}-beta`)
+                { dest: [6, 16], src: [8, 26] },
+                // m.assert|Equal(123, 123) => m.|assertEqual(123, 123)
+                { dest: [7, 16], src: [9, 26] }
+
+            ]);
+        });
+
         it('test full transpile', async () => {
             plugin.afterProgramCreate(program);
             // program.validate();
@@ -1576,15 +1632,18 @@ describe('RooibosPlugin', () => {
                 item = {
                 id: "item"
                 }
-
                 m.currentAssertLineNumber = 7
                 m._expectNotCalled(item, "getFunction", item, "item")
-                if m.currentResult?.isFail = true then m.done() : return invalid
-
-
+                if m.currentResult?.isFail = true then
+                m.done()
+                return invalid
+                end if
                 m.currentAssertLineNumber = 8
                 m._expectNotCalled(item, "getFunction", item, "item")
-                if m.currentResult?.isFail = true then m.done() : return invalid
+                if m.currentResult?.isFail = true then
+                m.done()
+                return invalid
+                end if
             `);
 
             let a = getContents('rooibos/RuntimeConfig.brs');
@@ -1712,6 +1771,57 @@ describe('RooibosPlugin', () => {
             console.log('done');
         });
     });
+
+
+    /**
+     * Test that the sourcemaps are generated properly and map each token back to their original location.
+     *
+     * Keep in mind, the expectedText is trimmed, so as you're calculating the dest positions, brighterscript will probably omit the leading newline when transpiled,
+     * so your dest lines might need to be 1 line smaller. Also, brighterscript does not honor source indentation, so your column numbers are probably going to be
+     * based on the standard brighterscript formatting.
+     * @param text the text to parse
+     * @param expectedText the expected output text
+     * @param expectedLocations an array of zero-based line and column arrays
+     */
+    async function testSourcemapLocations(text: string, expectedText: string, expectedLocations: Array<{ src: [number, number]; dest: [number, number] }>) {
+        program.options.sourceMap = true;
+        builder.options.sourceMap = true;
+        fsExtra.outputFileSync(`${_rootDir}/source/test.spec.bs`, text);
+        //set the file
+        program.setFile('source/test.spec.bs', text);
+
+        program.validate();
+        //build the project
+        await builder.transpile();
+
+        const actualContents = getContents('test.spec.brs');
+        expect(actualContents).to.eql(undent`${expectedText}`);
+
+        const map = fsExtra.readFileSync(s`${_stagingFolderPath}/source/test.spec.brs.map`).toString();
+
+        //load the source map
+        await SourceMapConsumer.with(map, null, (consumer) => {
+            expect(
+                expectedLocations.map((x) => {
+                    let originalPosition = consumer.originalPositionFor({
+                        //convert 0-based line to source-map 1-based line for the lookup
+                        line: x.dest[0] + 1,
+                        column: x.dest[1],
+                        bias: SourceMapConsumer.GREATEST_LOWER_BOUND
+                    });
+                    return Position.create(
+                        //convert 1-based source-map line to 0-based for the test
+                        originalPosition.line - 1,
+                        originalPosition.column
+                    );
+                })
+            ).to.eql(
+                expectedLocations.map(
+                    x => Position.create(x.src[0], x.src[1])
+                )
+            );
+        });
+    }
 });
 
 function getContents(filename: string) {
@@ -1747,4 +1857,3 @@ function getTestSubContents(trimEveryLine = false) {
 function trimLeading(text: string) {
     return text.split('\n').map((line) => line.trimStart()).join('\n');
 }
-
diff --git a/package-lock.json b/package-lock.json
diff --git a/rooibos.code-workspace b/rooibos.code-workspace
@@ -1,5 +1,5 @@
 {
-	"folders": [
+  "folders": [
     {
       "path": "bsc-plugin"
     },
@@ -16,17 +16,21 @@
   "settings": {
     "jira-plugin.workingProject": "",
     "workbench.colorCustomizations": {
-      "statusBar.background": "#b3b024",
+      "statusBar.background": "#f1ee2f",
+      "statusBar.foreground": "#000000",
       "statusBar.debuggingBackground": "#f1ee2f",
+      "statusBar.debuggingForeground": "#000000",
       "panelTitle.activeBorder": "#ff0000",
       "activityBar.background": "#f1ee2f",
       "activityBar.foreground": "#1b1b1a",
       "activityBar.activeBackground": "#b3b024",
-      "titleBar.activeBackground": "#b3b024",
-      "titleBar.activeForeground": "#F9F9F8"
+      "titleBar.activeBackground": "#f1ee2f",
+      "titleBar.activeForeground": "#000000",
+      "titleBar.inactiveBackground": "#f1ee2f",
+      "titleBar.inactiveForeground": "#000000"
     },
     "cSpell.words": [
       "Inifnite"
     ]
   }
-}
+}