From 9eea48946d6464adeda401964d95ebe7fb3dabec Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Sat, 6 Jan 2024 02:57:36 -0500 Subject: [PATCH] Fix some sourcemap transpile issues (#249) very thoughtful of you to change the colors: I get (no pun intended) blind to these sometimes. Thanks. --- bsc-plugin/package.json | 6 +- bsc-plugin/src/lib/rooibos/MockUtil.ts | 4 +- bsc-plugin/src/lib/rooibos/TestGroup.ts | 26 ++--- bsc-plugin/src/lib/rooibos/Utils.ts | 19 +--- bsc-plugin/src/plugin.spec.ts | 123 ++++++++++++++++++++++-- package-lock.json | 2 + rooibos.code-workspace | 14 ++- 7 files changed, 145 insertions(+), 49 deletions(-) diff --git a/bsc-plugin/package.json b/bsc-plugin/package.json index fc65e42b..ceefc7f4 100644 --- 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 index 66668ffb..0af49936 100644 --- 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 index 91e34952..b09da4c2 100644 --- 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 index 277ee654..7299297b 100644 --- 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 index c6594ca9..45dc4d9e 100644 --- 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 index fdfc9bdc..a2b1adee 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,10 +1,12 @@ { "name": "rooibos", + "version": "1.0.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "rooibos", + "version": "1.0.0", "license": "MIT", "devDependencies": { "@rokucommunity/bslint": "^0.5.0", diff --git a/rooibos.code-workspace b/rooibos.code-workspace index 2028e1fa..0ec85f1a 100644 --- 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" ] } -} \ No newline at end of file +}