Implement source map support#2387
Conversation
aardvark179
left a comment
aardvark179
left a comment
There was a problem hiding this comment.
I think the code looks good, but could we get a couple of test cases to represent the most common sorts of map?
- A source map which represents minification, i.e. it has collapsed all source lines to one minified line.
- A source map which represents transpiration, i.e. it has expanded each line to potentially multiple lines, but not collapsed any lines.
I know we are currently limited by our line maps, and that's a good reason for source maps not being applied lazily, but I would be interested if you think there is a path to not evaluating them until they are really needs for either error reporting or debugging
| * @param sourceLine 1-indexed line number in the original source | ||
| * @return the line text, or {@code null} if the line is out of range or unavailable | ||
| */ | ||
| String getSourceLineText(int sourceLine); |
There was a problem hiding this comment.
So, I'm trying to work out if this really makes sense. If you have an error in your original source then that should have been caught during whatever transformation process is being done, and if that transformation process introduced the error then that's the source you probably want to see.
There was a problem hiding this comment.
Yeah, realistically I doubt that you'd get an error in Rhino if your code has gone through the transpiler already. But, this is the logic we have in our fork and I thought it was simple enough to be worth keeping.
andreabergia
force-pushed
the
scratch/ab/source-maps-1
branch
2 times, most recently
from
cdf60e4 to
e26ccce
Compare
Added |
andreabergia
force-pushed
the
scratch/ab/source-maps-1
branch
from
e26ccce to
26429dd
Compare
|
The CI failure seems to be just an environmental transitory problem:
|
andreabergia
force-pushed
the
scratch/ab/source-maps-1
branch
from
26429dd to
d37b9a1
Compare
| 'org.mozilla.javascript.lc.type', | ||
| 'org.mozilla.javascript.optimizer', | ||
| 'org.mozilla.javascript.serialize', | ||
| 'org.mozilla.javascript.sourcemap', |
There was a problem hiding this comment.
I realize I'm the one who did NOT want BND to manage both OSGi modules and Java modules, but I believe that if you're going to export this new package in OSGi, you should probably export it in module-info.java as well unless you're sure it'll never be used outside this module.
There was a problem hiding this comment.
Fixed that (and rebased)
Adds an abstract `SourceMapper` interface. Callers attach a mapper via `ScriptCompileSpec`/`FunctionCompileSpec` builders; the mapper is held on `CompilerEnvirons`. It is consulted during code generation in both backends (`BodyCodegen` and `CodeGenerator`) so emitted line numbers refer to the original source - i.e. this means that features such as debugger breakpoints or stack traces come for free. Parser error reporting and the debugger source handoff also use the mapper. Null mappings cause the corresponding line entry to be skipped in both LineNumberTable and Icode_LINE. Tests cover both backends where applicable (via `Utils.runWithAllModes`), although there are some interpreter-only tests. `InterpreterIcodeCapture` was added to reduces duplication with `InterpreterBytecodeDumpTest`.
The test represents a minification of:
```js
var x = 1;
throw new Error("oops");
```
So it squeezes two lines into one.
Expands the lines from 1 to 2
andreabergia
force-pushed
the
scratch/ab/source-maps-1
branch
from
d37b9a1 to
050e195
Compare
|
OK, thanks! |
4 participants
Stacked on top of #2388. This is basically the internal implementation that we have had in ServiceNow for some years now, slightly cleaned up.
Adds an abstract
SourceMapperinterface. Callers attach a mapper viaScriptCompileSpec/FunctionCompileSpecbuilders; the mapper is heldon
CompilerEnvirons.It is consulted during code generation in both backends (
BodyCodegenand
CodeGenerator) so emitted line numbers refer to the originalsource - i.e. this means that features such as debugger breakpoints or
stack traces come for free.
Parser error reporting and the debugger source handoff also use the
mapper. Null mappings cause the corresponding line entry to be skipped
in both LineNumberTable and Icode_LINE.
Tests cover both backends where applicable (via
Utils.runWithAllModes), although there are some interpreter-onlytests.
InterpreterIcodeCapturewas added to reduces duplication withInterpreterBytecodeDumpTest.The next step will be implementing a fully spec-compliant source mapper parser that uses the official test suite.