Skip to content

Commit 4adc7b0

Browse files
authored
Merge pull request microsoft#1125 from Microsoft/octogonz/ae-source-maps
[api-extractor] When reporting error line numbers, use source maps to find the original .ts file
2 parents bbf2b4c + e9cbb25 commit 4adc7b0

7 files changed

Lines changed: 274 additions & 33 deletions

File tree

apps/api-extractor/package.json

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,17 @@
3838
"@types/z-schema": "3.16.31",
3939
"colors": "~1.2.1",
4040
"lodash": "~4.17.5",
41+
"resolve": "1.8.1",
42+
"source-map": "~0.6.1",
4143
"typescript": "~3.1.6",
42-
"z-schema": "~3.18.3",
43-
"resolve": "1.8.1"
44+
"z-schema": "~3.18.3"
4445
},
4546
"devDependencies": {
47+
"@microsoft/node-library-build": "6.0.29",
4648
"@microsoft/rush-stack-compiler-3.2": "0.2.2",
47-
"tslint-microsoft-contrib": "~5.2.1",
49+
"@types/jest": "23.3.11",
4850
"@types/lodash": "4.14.116",
4951
"gulp": "~3.9.1",
50-
"@microsoft/node-library-build": "6.0.29",
51-
"@types/jest": "23.3.11"
52+
"tslint-microsoft-contrib": "~5.2.1"
5253
}
5354
}

apps/api-extractor/src/collector/MessageRouter.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
} from '../api/IExtractorConfig';
2222
import { AedocDefinitions } from '../aedoc/AedocDefinitions';
2323
import { ILogger } from '../api/ILogger';
24+
import { SourceMapper } from './SourceMapper';
2425

2526
interface IReportingRule {
2627
logLevel: ExtractorMessageLogLevel;
@@ -37,6 +38,8 @@ export class MessageRouter {
3738
// Messages that got written to the API review file
3839
private readonly _messagesAddedToApiReviewFile: Set<ExtractorMessage>;
3940

41+
private readonly _sourceMapper: SourceMapper;
42+
4043
// Normalized representation of the routing rules from api-extractor.json
4144
private _reportingRuleByMessageId: Map<string, IReportingRule> = new Map<string, IReportingRule>();
4245
private _compilerDefaultRule: IReportingRule = { logLevel: ExtractorMessageLogLevel.None,
@@ -50,6 +53,7 @@ export class MessageRouter {
5053
this._messages = [];
5154
this._associatedMessagesForAstDeclaration = new Map<AstDeclaration, ExtractorMessage[]>();
5255
this._messagesAddedToApiReviewFile = new Set<ExtractorMessage>();
56+
this._sourceMapper = new SourceMapper();
5357

5458
this._applyMessagesConfig(messagesConfig);
5559
}
@@ -151,6 +155,8 @@ export class MessageRouter {
151155
options.sourceFileColumn = lineAndCharacter.character + 1;
152156
}
153157

158+
// NOTE: Since compiler errors pertain to issues specific to the .d.ts files,
159+
// we do not apply source mappings for them.
154160
this._messages.push(new ExtractorMessage(options));
155161
}
156162

@@ -169,7 +175,7 @@ export class MessageRouter {
169175

170176
const extractorMessage: ExtractorMessage = this.addAnalyzerIssueForPosition(
171177
messageId, messageText, astDeclaration.declaration.getSourceFile(),
172-
astDeclaration.declaration.pos);
178+
astDeclaration.declaration.getStart());
173179

174180
this._associateMessageWithAstDeclaration(extractorMessage, astDeclaration);
175181
}
@@ -185,14 +191,17 @@ export class MessageRouter {
185191
const lineAndCharacter: ts.LineAndCharacter = sourceFile.getLineAndCharacterOfPosition(
186192
message.textRange.pos);
187193

188-
const extractorMessage: ExtractorMessage = new ExtractorMessage({
194+
const options: IExtractorMessageOptions = {
189195
category: ExtractorMessageCategory.TSDoc,
190196
messageId: message.messageId,
191197
text: message.unformattedText,
192198
sourceFilePath: sourceFile.fileName,
193199
sourceFileLine: lineAndCharacter.line + 1,
194200
sourceFileColumn: lineAndCharacter.character + 1
195-
});
201+
};
202+
203+
this._sourceMapper.updateExtractorMessageOptions(options);
204+
const extractorMessage: ExtractorMessage = new ExtractorMessage(options);
196205

197206
if (astDeclaration) {
198207
this._associateMessageWithAstDeclaration(extractorMessage, astDeclaration);
@@ -236,7 +245,9 @@ export class MessageRouter {
236245
sourceFileColumn: lineAndCharacter.character + 1
237246
};
238247

248+
this._sourceMapper.updateExtractorMessageOptions(options);
239249
const extractorMessage: ExtractorMessage = new ExtractorMessage(options);
250+
240251
this._messages.push(extractorMessage);
241252
return extractorMessage;
242253
}
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import * as path from 'path';
5+
import { SourceMapConsumer, RawSourceMap, MappingItem, Position } from 'source-map';
6+
import { IExtractorMessageOptions } from '../api/ExtractorMessage';
7+
import { FileSystem, InternalError, JsonFile, NewlineKind } from '@microsoft/node-core-library';
8+
9+
interface ISourceMap {
10+
sourceMapConsumer: SourceMapConsumer;
11+
12+
// SourceMapConsumer.originalPositionFor() is useless because the mapping contains numerous gaps,
13+
// and the API provides no way to find the nearest match. So instead we extract all the mapping items
14+
// and search them using SourceMapper._findNearestMappingItem().
15+
mappingItems: MappingItem[];
16+
}
17+
18+
interface IOriginalFileInfo {
19+
// Whether the .ts file exists
20+
fileExists: boolean;
21+
22+
// This is used to check whether the guessed position is out of bounds.
23+
// Since column/line numbers are 1-based, the 0th item in this array is unused.
24+
maxColumnForLine: number[];
25+
}
26+
27+
export class SourceMapper {
28+
// Map from .d.ts file path --> ISourceMap if a source map was found, or null if not found
29+
private _sourceMapByFilePath: Map<string, ISourceMap | null>
30+
= new Map<string, ISourceMap | null>();
31+
32+
// Cache the FileSystem.exists() result for mapped .ts files
33+
private _originalFileInfoByPath: Map<string, IOriginalFileInfo> = new Map<string, IOriginalFileInfo>();
34+
35+
/**
36+
* If the `IExtractorMessageOptions` refers to a `.d.ts` file, look for a `.d.ts.map` and
37+
* if possible update the coordinates to refer to the original `.ts` file.
38+
*/
39+
public updateExtractorMessageOptions(options: IExtractorMessageOptions): void {
40+
if (!options.sourceFilePath) {
41+
return;
42+
}
43+
44+
if (!FileSystem.exists(options.sourceFilePath)) {
45+
// Sanity check
46+
throw new InternalError('The referenced path was not found: ' + options.sourceFilePath);
47+
}
48+
49+
let sourceMap: ISourceMap | null | undefined = this._sourceMapByFilePath.get(options.sourceFilePath);
50+
51+
if (sourceMap === undefined) {
52+
// Normalize the path and redo the lookup
53+
const normalizedPath: string = FileSystem.getRealPath(options.sourceFilePath);
54+
55+
sourceMap = this._sourceMapByFilePath.get(normalizedPath);
56+
if (sourceMap !== undefined) {
57+
// Copy the result from the normalized to the non-normalized key
58+
this._sourceMapByFilePath.set(options.sourceFilePath, sourceMap);
59+
} else {
60+
// Given "folder/file.d.ts", check for a corresponding "folder/file.d.ts.map"
61+
const sourceMapPath: string = normalizedPath + '.map';
62+
if (FileSystem.exists(sourceMapPath)) {
63+
// Load up the source map
64+
const rawSourceMap: RawSourceMap = JsonFile.load(sourceMapPath) as RawSourceMap;
65+
66+
const sourceMapConsumer: SourceMapConsumer = new SourceMapConsumer(rawSourceMap);
67+
const mappingItems: MappingItem[] = [];
68+
69+
// Extract the list of mapping items
70+
sourceMapConsumer.eachMapping(
71+
(mappingItem: MappingItem) => {
72+
mappingItems.push({
73+
...mappingItem,
74+
// The "source-map" package inexplicably uses 1-based line numbers but 0-based column numbers.
75+
// Fix that up proactively so we don't have to deal with it later.
76+
generatedColumn: mappingItem.generatedColumn + 1,
77+
originalColumn: mappingItem.originalColumn + 1
78+
});
79+
},
80+
this,
81+
SourceMapConsumer.GENERATED_ORDER
82+
);
83+
84+
sourceMap = { sourceMapConsumer, mappingItems};
85+
} else {
86+
// No source map for this filename
87+
sourceMap = null; // tslint:disable-line:no-null-keyword
88+
}
89+
90+
this._sourceMapByFilePath.set(normalizedPath, sourceMap);
91+
if (options.sourceFilePath !== normalizedPath) {
92+
// Add both keys to the map
93+
this._sourceMapByFilePath.set(options.sourceFilePath, sourceMap);
94+
}
95+
}
96+
}
97+
98+
if (sourceMap === null) {
99+
// No source map for this filename
100+
return;
101+
}
102+
103+
// Make sure sourceFileLine and sourceFileColumn are defined
104+
if (options.sourceFileLine === undefined) {
105+
options.sourceFileLine = 1;
106+
}
107+
if (options.sourceFileColumn === undefined) {
108+
options.sourceFileColumn = 1;
109+
}
110+
111+
const nearestMappingItem: MappingItem | undefined = SourceMapper._findNearestMappingItem(sourceMap.mappingItems,
112+
{
113+
line: options.sourceFileLine,
114+
column: options.sourceFileColumn
115+
}
116+
);
117+
118+
if (nearestMappingItem === undefined) {
119+
// No mapping for this location
120+
return;
121+
}
122+
123+
const mappedFilePath: string = path.resolve(path.dirname(options.sourceFilePath), nearestMappingItem.source);
124+
125+
// Does the mapped filename exist? Use a cache to remember the answer.
126+
let originalFileInfo: IOriginalFileInfo | undefined = this._originalFileInfoByPath.get(mappedFilePath);
127+
if (originalFileInfo === undefined) {
128+
originalFileInfo = {
129+
fileExists: FileSystem.exists(mappedFilePath),
130+
maxColumnForLine: []
131+
};
132+
133+
if (originalFileInfo.fileExists) {
134+
// Read the file and measure the length of each line
135+
originalFileInfo.maxColumnForLine =
136+
FileSystem.readFile(mappedFilePath, { convertLineEndings: NewlineKind.Lf })
137+
.split('\n')
138+
.map(x => x.length + 1); // +1 since columns are 1-based
139+
originalFileInfo.maxColumnForLine.unshift(0); // Extra item since lines are 1-based
140+
}
141+
142+
this._originalFileInfoByPath.set(mappedFilePath, originalFileInfo);
143+
}
144+
145+
if (!originalFileInfo.fileExists) {
146+
// Don't translate coordinates to a file that doesn't exist
147+
return;
148+
}
149+
150+
// The nearestMappingItem anchor may be above/left of the real position, due to gaps in the mapping. Calculate
151+
// the delta and apply it to the original position.
152+
const guessedPosition: Position = {
153+
line: nearestMappingItem.originalLine + options.sourceFileLine - nearestMappingItem.generatedLine,
154+
column: nearestMappingItem.originalColumn + options.sourceFileColumn - nearestMappingItem.generatedColumn
155+
};
156+
157+
// Verify that the result is not out of bounds, in cause our heuristic failed
158+
if (guessedPosition.line >= 1
159+
&& guessedPosition.line < originalFileInfo.maxColumnForLine.length
160+
&& guessedPosition.column >= 1
161+
&& guessedPosition.column <= originalFileInfo.maxColumnForLine[guessedPosition.line]) {
162+
163+
options.sourceFilePath = mappedFilePath;
164+
options.sourceFileLine = guessedPosition.line;
165+
options.sourceFileColumn = guessedPosition.column;
166+
} else {
167+
// The guessed position was out of bounds, so use the nearestMappingItem position instead.
168+
options.sourceFilePath = mappedFilePath;
169+
options.sourceFileLine = nearestMappingItem.originalLine;
170+
options.sourceFileColumn = nearestMappingItem.originalColumn;
171+
}
172+
}
173+
174+
// The `mappingItems` array is sorted by generatedLine/generatedColumn (GENERATED_ORDER).
175+
// The _findNearestMappingItem() lookup is a simple binary search that returns the previous item
176+
// if there is no exact match.
177+
private static _findNearestMappingItem(mappingItems: MappingItem[], position: Position): MappingItem | undefined {
178+
if (mappingItems.length === 0) {
179+
return undefined;
180+
}
181+
182+
let startIndex: number = 0;
183+
let endIndex: number = mappingItems.length - 1;
184+
185+
while (startIndex <= endIndex) {
186+
const middleIndex: number = startIndex + Math.floor((endIndex - startIndex) / 2);
187+
188+
const diff: number = SourceMapper._compareMappingItem(mappingItems[middleIndex], position);
189+
190+
if (diff < 0) {
191+
startIndex = middleIndex + 1;
192+
} if (diff > 0) {
193+
endIndex = middleIndex - 1;
194+
} else {
195+
// Exact match
196+
return mappingItems[middleIndex];
197+
}
198+
}
199+
200+
// If we didn't find an exact match, then endIndex < startIndex.
201+
// Take endIndex because it's the smaller value.
202+
return mappingItems[endIndex];
203+
}
204+
205+
private static _compareMappingItem(mappingItem: MappingItem, position: Position): number {
206+
const diff: number = mappingItem.generatedLine - position.line;
207+
if (diff !== 0) {
208+
return diff;
209+
}
210+
return mappingItem.generatedColumn - position.column;
211+
}
212+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/api-extractor",
5+
"comment": "Error messages now cite the original .ts source file, if a source map is present. (To enable this, specify `\"declarationMap\": true` in tsconfig.json.)",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@microsoft/api-extractor",
10+
"email": "4673363+octogonz@users.noreply.github.com"
11+
}
Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
// DO NOT ADD COMMENTS IN THIS FILE. They will be lost when the Rush tool resaves it.
22
{
33
"$schema": "https://developer.microsoft.com/json-schemas/rush/v5/approved-packages.schema.json",
4-
"packages": [
5-
{
6-
"name": "@microsoft/rush-stack-compiler-3.3",
7-
"allowedCategories": [ "tests" ]
8-
}
9-
]
4+
"packages": []
105
}

common/config/rush/nonbrowser-approved-packages.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@
8686
"name": "@microsoft/rush-stack-compiler-3.2",
8787
"allowedCategories": [ "libraries", "tests" ]
8888
},
89+
{
90+
"name": "@microsoft/rush-stack-compiler-3.3",
91+
"allowedCategories": [ "tests" ]
92+
},
8993
{
9094
"name": "@microsoft/rush-stack-compiler-shared",
9195
"allowedCategories": [ "libraries" ]
@@ -434,6 +438,10 @@
434438
"name": "sinon-chai",
435439
"allowedCategories": [ "libraries" ]
436440
},
441+
{
442+
"name": "source-map",
443+
"allowedCategories": [ "libraries" ]
444+
},
437445
{
438446
"name": "strict-uri-encode",
439447
"allowedCategories": [ "libraries" ]

0 commit comments

Comments
 (0)