refactor(localize): handle location endings correctly (#32912)

Previously source locations required an ending position but this was not
being computed effectively. Now ending position is optional and it is
computed from an `endPath` passed to `getLocation()`.

PR Close #32912
This commit is contained in:
Pete Bacon Darwin 2020-06-05 13:51:13 +01:00 committed by Andrew Kushnir
parent 07a07e34bc
commit ddb0a4e2e5
4 changed files with 53 additions and 26 deletions

View File

@ -5,6 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {AbsoluteFsPath, relative, resolve} from '@angular/compiler-cli/src/ngtsc/file_system';
import {ɵisMissingTranslationError, ɵmakeTemplateObject, ɵParsedTranslation, ɵSourceLocation, ɵtranslate} from '@angular/localize';
import {NodePath} from '@babel/traverse';
import * as t from '@babel/types';
@ -347,15 +348,31 @@ export function buildCodeFrameError(path: NodePath, e: BabelParseError): string
return `${filename}: ${message}`;
}
export function getLocation(path: NodePath): ɵSourceLocation|undefined {
const location = path.node.loc;
const file = path.hub.file.opts.filename;
if (!location || !file) {
export function getLocation(startPath: NodePath, endPath?: NodePath): ɵSourceLocation|undefined {
const startLocation = startPath.node.loc;
const file = getFileFromPath(startPath);
if (!startLocation || !file) {
return undefined;
}
// Note we clone the `start` and `end` objects so that their prototype chains,
// from Babel, do not leak into our code.
return {start: {...location.start}, end: {...location.end}, file};
const endLocation =
endPath && getFileFromPath(endPath) === file && endPath.node.loc || startLocation;
return {
start: getLineAndColumn(startLocation.start),
end: getLineAndColumn(endLocation.end),
file
};
}
function getFileFromPath(path: NodePath|undefined): AbsoluteFsPath|null {
const opts = path?.hub.file.opts;
return opts?.filename ?
resolve(opts.generatorOpts.sourceRoot, relative(opts.cwd, opts.filename)) :
null;
}
function getLineAndColumn(loc: {line: number, column: number}): {line: number, column: number} {
// Note we want 0-based line numbers but Babel returns 1-based.
return {line: loc.line - 1, column: loc.column};
}

View File

@ -5,6 +5,8 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
import {ɵmakeTemplateObject} from '@angular/localize';
import {NodePath, TransformOptions, transformSync} from '@babel/core';
import generate from '@babel/generator';
@ -195,23 +197,27 @@ describe('utils', () => {
});
});
describe('getLocation()', () => {
it('should return a plain object containing the start, end and file of a NodePath', () => {
const taggedTemplate =
getTaggedTemplate('const x = $localize ``;', {filename: 'src/test.js'});
const location = getLocation(taggedTemplate)!;
expect(location).toBeDefined();
expect(location.start).toEqual({line: 1, column: 10});
expect(location.start.constructor.name).toEqual('Object');
expect(location.end).toEqual({line: 1, column: 22});
expect(location.end.constructor.name).toEqual('Object');
expect(location.file).toContain('src/test.js');
});
runInEachFileSystem(() => {
describe('getLocation()', () => {
it('should return a plain object containing the start, end and file of a NodePath', () => {
const taggedTemplate = getTaggedTemplate('const x = $localize `message`;', {
filename: 'src/test.js',
sourceRoot: '/root',
});
const location = getLocation(taggedTemplate)!;
expect(location).toBeDefined();
expect(location.start).toEqual({line: 0, column: 10});
expect(location.start.constructor.name).toEqual('Object');
expect(location.end).toEqual({line: 0, column: 29});
expect(location.end?.constructor.name).toEqual('Object');
expect(location.file).toEqual(absoluteFrom('/root/src/test.js'));
});
it('should return undefined if the NodePath has no filename', () => {
const taggedTemplate = getTaggedTemplate('const x = $localize ``;');
const location = getLocation(taggedTemplate)!;
expect(location).toBeUndefined();
it('should return `undefined` if the NodePath has no filename', () => {
const taggedTemplate = getTaggedTemplate('const x = $localize ``;', {sourceRoot: '/root'});
const location = getLocation(taggedTemplate);
expect(location).toBeUndefined();
});
});
});
});

View File

@ -13,5 +13,6 @@ ts_library(
module_name = "@angular/localize/src/utils",
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/file_system",
],
)

View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {computeMsgId} from '@angular/compiler';
import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
import {BLOCK_MARKER, ID_SEPARATOR, LEGACY_ID_INDICATOR, MEANING_SEPARATOR} from './constants';
@ -39,12 +40,14 @@ export type TargetMessage = string;
export type MessageId = string;
/**
* The location of the message
* The location of the message in the source file.
*
* The `line` and `column` values for the `start` and `end` properties are zero-based.
*/
export interface SourceLocation {
start: {line: number, column: number};
end: {line: number, column: number};
file: string;
file: AbsoluteFsPath;
}
/**