fix(language-service): fix calculation of pipe spans (#35986)

This commit accomplishes two tasks:

- Fixes the span of queried pipes to only be applied on pipe names
- By consequence, fixes how pipes are located in arguments (previously,
  pipes with arguments could not be found because the span of a pipe
  uses a relative span, while the template position is absolute)

The screenshots attached to the PR for this commit demonstrate the
change.

Closes https://github.com/angular/vscode-ng-language-service/issues/677

PR Close #35986
This commit is contained in:
ayazhafiz 2020-03-09 22:16:08 -07:00 committed by Matias Niemelä
parent a73e125c04
commit 406419bc0f
4 changed files with 59 additions and 13 deletions

View File

@ -96,6 +96,14 @@ export function getExpressionCompletions(
return result && result.values(); return result && result.values();
} }
/**
* Retrieves the expression symbol at a particular position in a template.
*
* @param scope symbols in scope of the template
* @param ast template AST
* @param position absolute location in template to retrieve symbol at
* @param query type symbol query for the template scope
*/
export function getExpressionSymbol( export function getExpressionSymbol(
scope: SymbolTable, ast: AST, position: number, scope: SymbolTable, ast: AST, position: number,
query: SymbolQuery): {symbol: Symbol, span: Span}|undefined { query: SymbolQuery): {symbol: Symbol, span: Span}|undefined {
@ -129,14 +137,19 @@ export function getExpressionSymbol(
span = ast.span; span = ast.span;
}, },
visitPipe(ast) { visitPipe(ast) {
if (position >= ast.exp.span.end && if (inSpan(position, ast.nameSpan, /* exclusive */ true)) {
(!ast.args || !ast.args.length || position < (<AST>ast.args[0]).span.start)) {
// We are in a position a pipe name is expected. // We are in a position a pipe name is expected.
const pipes = query.getPipes(); const pipes = query.getPipes();
if (pipes) { symbol = pipes.get(ast.name);
symbol = pipes.get(ast.name);
span = ast.span; // `nameSpan` is an absolute span, but the span expected by the result of this method is
} // relative to the start of the expression.
// TODO(ayazhafiz): migrate to only using absolute spans
const offset = ast.sourceSpan.start - ast.span.start;
span = {
start: ast.nameSpan.start - offset,
end: ast.nameSpan.end - offset,
};
} }
}, },
visitPrefixNot(ast) {}, visitPrefixNot(ast) {},

View File

@ -234,22 +234,19 @@ describe('definitions', () => {
it('should be able to find a pipe', () => { it('should be able to find a pipe', () => {
const fileName = mockHost.addCode(` const fileName = mockHost.addCode(`
@Component({ @Component({
template: '<div *ngIf="~{start-my}input | «async»~{end-my}"></div>' template: '<div *ngIf="input | «async»"></div>'
}) })
export class MyComponent { export class MyComponent {
input: EventEmitter; input: EventEmitter;
}`); }`);
// Get the marker for «test» in the code added above. // Get the marker for «async» in the code added above.
const marker = mockHost.getReferenceMarkerFor(fileName, 'async'); const marker = mockHost.getReferenceMarkerFor(fileName, 'async');
const result = ngService.getDefinitionAndBoundSpan(fileName, marker.start); const result = ngService.getDefinitionAndBoundSpan(fileName, marker.start);
expect(result).toBeDefined(); expect(result).toBeDefined();
const {textSpan, definitions} = result !; const {textSpan, definitions} = result !;
expect(textSpan).toEqual(marker);
// Get the marker for bounded text in the code added above
const boundedText = mockHost.getLocationMarkerFor(fileName, 'my');
expect(textSpan).toEqual(boundedText);
expect(definitions).toBeDefined(); expect(definitions).toBeDefined();
expect(definitions !.length).toBe(4); expect(definitions !.length).toBe(4);
@ -263,6 +260,28 @@ describe('definitions', () => {
} }
}); });
// https://github.com/angular/vscode-ng-language-service/issues/677
it('should be able to find a pipe with arguments', () => {
mockHost.override(TEST_TEMPLATE, `{{birthday | «date»: "MM/dd/yy"}}`);
const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'date');
const result = ngService.getDefinitionAndBoundSpan(TEST_TEMPLATE, marker.start);
expect(result).toBeDefined();
const {textSpan, definitions} = result !;
expect(textSpan).toEqual(marker);
expect(definitions).toBeDefined();
expect(definitions !.length).toBe(1);
const refFileName = '/node_modules/@angular/common/common.d.ts';
for (const def of definitions !) {
expect(def.fileName).toBe(refFileName);
expect(def.name).toBe('date');
expect(def.kind).toBe('pipe');
}
});
describe('in structural directive', () => { describe('in structural directive', () => {
it('should be able to find the directive', () => { it('should be able to find the directive', () => {
mockHost.override( mockHost.override(

View File

@ -138,6 +138,19 @@ describe('hover', () => {
expect(toText(displayParts)).toBe('(property) TemplateReference.heroes: Hero[]'); expect(toText(displayParts)).toBe('(property) TemplateReference.heroes: Hero[]');
}); });
it('should work for pipes', () => {
mockHost.override(TEST_TEMPLATE, `
<p>The hero's birthday is {{birthday | «date»: "MM/dd/yy"}}</p>`);
const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'date');
const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start);
expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker);
expect(toText(displayParts))
.toBe(
'(pipe) date: (value: any, format?: string | undefined, timezone?: string | undefined, locale?: string | undefined) => string | null');
});
it('should work for the $any() cast function', () => { it('should work for the $any() cast function', () => {
const content = mockHost.override(TEST_TEMPLATE, '<div>{{$any(title)}}</div>'); const content = mockHost.override(TEST_TEMPLATE, '<div>{{$any(title)}}</div>');
const position = content.indexOf('$any'); const position = content.indexOf('$any');

View File

@ -171,6 +171,7 @@ export class TemplateReference {
anyValue: any; anyValue: any;
optional?: string; optional?: string;
myClick(event: any) {} myClick(event: any) {}
birthday = new Date();
} }
@Component({ @Component({