fix(source-maps): Use alternate source map bias only if the default returns null and the alternate does not. Remove as user option to reduce config confusion. (#303)

In an effort to reduce config options (and confusion), this removes the
sourceMapBias param from the launch config. The default source map bias
is 'leastUpperBound', which works with our published ts config in TS
Starter. This config is also used in the source-map unit tests. If
leastUpperBound fails, it will try greatestLowerBound but will not
switch to that as the preferred bias unless it succeeds.

Author: chmeyer-ms

package.json

@@ -128,10 +128,6 @@
                                 "type": "object",
                                 "description": "Module mapping for imports. Each key is an import name that will be mapped to the provided value. Used if modules are external (i.e. included as part of minecraft). Defaults to an empty object."
                             },
-                            "sourceMapBias": {
-                                "type": "string",
-                                "description": "The bias to use when mapping from generated code to source code. Can be either 'leastUpperBound' or 'greatestLowerBound'. Defaults to 'leastUpperBound'."
-                            },
                             "targetModuleUuid": {
                                 "type": "string",
                                 "description": "The script module uuid from the manifest.json of the Minecraft Add-On being debugged. Necessary if there are multiple Add-Ons active."

src/session.ts

@@ -74,7 +74,6 @@ interface IAttachRequestArguments extends DebugProtocol.AttachRequestArguments {
     port?: number;
     inputPort?: string;
     moduleMapping?: ModuleMapping;
-    sourceMapBias?: string;
     targetModuleUuid?: string;
     passcode?: string;
 }
@@ -135,7 +134,6 @@ export class Session extends DebugSession {
     private _generatedSourceRoot?: string;
     private _inlineSourceMap: boolean = false;
     private _moduleMapping?: ModuleMapping;
-    private _sourceMapBias?: string;
     private _targetModuleUuid?: string;
     private _clientProtocolVersion: number = ProtocolVersion._Unknown; // determined after connection
     private _minecraftCapabilities: MinecraftCapabilities = { supportsCommands: false, supportsProfiler: false };
@@ -301,7 +299,6 @@ export class Session extends DebugSession {
         this._sourceMapRoot = args.sourceMapRoot ? path.normalize(args.sourceMapRoot) : undefined;
         this._generatedSourceRoot = args.generatedSourceRoot ? path.normalize(args.generatedSourceRoot) : undefined;
         this._moduleMapping = args.moduleMapping;
-        this._sourceMapBias = args.sourceMapBias;
 
         // Listen or connect (default), depending on mode.
         // Attach makes more sense to use connect, but some MC platforms require using listen.
@@ -688,8 +685,7 @@ export class Session extends DebugSession {
             this._localRoot,
             this._sourceMapRoot,
             this._generatedSourceRoot,
-            this._inlineSourceMap,
-            this._sourceMapBias
+            this._inlineSourceMap
         );
 
         // watch for source map changes

src/source-maps.ts

@@ -178,26 +178,16 @@ export class SourceMaps {
     private _sourceMapRoot?: string;
     private _sourceMapCache: SourceMapCache;
     private _sourceMapBias: number = SourceMapConsumer.LEAST_UPPER_BOUND;
-    private _sourceMapBiasAlternate: number = SourceMapConsumer.GREATEST_LOWER_BOUND;
 
     public constructor(
         localRoot: string,
         sourceMapRoot?: string,
         generatedSourceRoot?: string,
-        inlineSourceMap: boolean = false,
-        sourceMapBias?: string
+        inlineSourceMap: boolean = false
     ) {
         this._localRoot = path.normalize(localRoot);
         this._sourceMapRoot = sourceMapRoot ? path.normalize(sourceMapRoot) : undefined;
         this._sourceMapCache = new SourceMapCache(this._sourceMapRoot, generatedSourceRoot, inlineSourceMap);
-        this._sourceMapBias =
-            sourceMapBias === 'greatestLowerBound'
-                ? SourceMapConsumer.GREATEST_LOWER_BOUND
-                : SourceMapConsumer.LEAST_UPPER_BOUND;
-        this._sourceMapBiasAlternate =
-            this._sourceMapBias === SourceMapConsumer.LEAST_UPPER_BOUND
-                ? SourceMapConsumer.GREATEST_LOWER_BOUND
-                : SourceMapConsumer.LEAST_UPPER_BOUND;
     }
 
     public clearCache() {
@@ -231,17 +221,20 @@ export class SourceMaps {
             source: mapInfo.preferAbsolute ? mapInfo.sourceAbsolutePath : mapInfo.originalSourceRelativePath,
             line: originalPosition.line,
             column: originalPosition.column,
-            bias: this._sourceMapBias,
+            bias: this._getSourceMapBias(),
         });
 
-        // if specified bias did not find a result, try the alternate
+        // default bias did not find a result, try the alternate
         if (generatedPosition.line === null) {
             generatedPosition = mapInfo.sourceMap.generatedPositionFor({
                 source: mapInfo.preferAbsolute ? mapInfo.sourceAbsolutePath : mapInfo.originalSourceRelativePath,
                 line: originalPosition.line,
                 column: originalPosition.column,
-                bias: this._sourceMapBiasAlternate,
+                bias: this._getAlternateSourceMapBias(),
             });
+            if (generatedPosition.line) {
+                this._switchSourceMapBias(); // alternate worked, make it primary
+            }
         }
 
         return generatedPosition;
@@ -260,16 +253,19 @@ export class SourceMaps {
             let originalPos = mapInfo.sourceMap.originalPositionFor({
                 line: generatedPosition.line,
                 column: generatedPosition.column,
-                bias: this._sourceMapBias,
+                bias: this._getSourceMapBias(),
             });
 
-            // if specified bias did not find a result, try the alternate
+            // default bias did not find a result, try the alternate
             if (originalPos.line === null) {
                 originalPos = mapInfo.sourceMap.originalPositionFor({
                     line: generatedPosition.line,
                     column: generatedPosition.column,
-                    bias: this._sourceMapBiasAlternate,
+                    bias: this._getAlternateSourceMapBias(),
                 });
+                if (originalPos.line) {
+                    this._switchSourceMapBias();
+                }
             }
 
             if (originalPos.line !== null && originalPos.column !== null && originalPos.source !== null) {
@@ -287,4 +283,21 @@ export class SourceMaps {
             `Could not map original position for ${generatedPosition.source} at line ${generatedPosition.line}.`
         );
     }
+
+    private _getSourceMapBias() {
+        return this._sourceMapBias;
+    }
+
+    private _getAlternateSourceMapBias() {
+        return this._sourceMapBias === SourceMapConsumer.LEAST_UPPER_BOUND
+            ? SourceMapConsumer.GREATEST_LOWER_BOUND
+            : SourceMapConsumer.LEAST_UPPER_BOUND;
+    }
+
+    private _switchSourceMapBias() {
+        this._sourceMapBias =
+            this._sourceMapBias === SourceMapConsumer.LEAST_UPPER_BOUND
+                ? SourceMapConsumer.GREATEST_LOWER_BOUND
+                : SourceMapConsumer.LEAST_UPPER_BOUND;
+    }
 }