Skip to content

Commit

Permalink
fix(language-service): break the hover/definitions for two-way binding (
Browse files Browse the repository at this point in the history
#34564)

PR Close #34564
  • Loading branch information
ivanwonder authored and atscott committed Jan 14, 2020
1 parent fe19327 commit eb5c20c
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 5 deletions.
27 changes: 22 additions & 5 deletions packages/language-service/src/locate_symbol.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST, Attribute, BoundDirectivePropertyAst, BoundEventAst, CompileTypeSummary, CssSelector, DirectiveAst, ElementAst, SelectorMatcher, TemplateAstPath, tokenReference} from '@angular/compiler';
import {AST, Attribute, BoundDirectivePropertyAst, BoundEventAst, CompileTypeSummary, CssSelector, DirectiveAst, ElementAst, RecursiveTemplateAstVisitor, SelectorMatcher, TemplateAst, TemplateAstPath, templateVisitAll, tokenReference} from '@angular/compiler';

import {AstResult} from './common';
import {getExpressionScope} from './expression_diagnostics';
Expand Down Expand Up @@ -148,14 +148,31 @@ function findAttribute(info: AstResult, position: number): Attribute|undefined {
return path.first(Attribute);
}

function findParentOfDirectivePropertyAst(ast: TemplateAst[], binding: BoundDirectivePropertyAst) {
let res: DirectiveAst|undefined;
const visitor = new class extends RecursiveTemplateAstVisitor {
visitDirective(ast: DirectiveAst) {
const result = this.visitChildren(ast, visit => { visit(ast.inputs); });
return result;
}
visitDirectiveProperty(ast: BoundDirectivePropertyAst, context: any) {
if (ast === binding) {
res = context;
}
}
};
templateVisitAll(visitor, ast);
return res;
}

function findInputBinding(
info: AstResult, path: TemplateAstPath, binding: BoundDirectivePropertyAst): Symbol|undefined {
const directive = path.parentOf(path.tail);
if (directive instanceof DirectiveAst) {
const invertedInput = invertMap(directive.directive.inputs);
const directiveAst = findParentOfDirectivePropertyAst(info.templateAst, binding);
if (directiveAst) {
const invertedInput = invertMap(directiveAst.directive.inputs);
const fieldName = invertedInput[binding.templateName];
if (fieldName) {
const classSymbol = info.template.query.getTypeSymbol(directive.directive.type.reference);
const classSymbol = info.template.query.getTypeSymbol(directiveAst.directive.type.reference);
if (classSymbol) {
return classSymbol.members().get(fieldName);
}
Expand Down
36 changes: 36 additions & 0 deletions packages/language-service/test/definitions_spec.ts
Expand Up @@ -291,6 +291,42 @@ describe('definitions', () => {
// Not asserting the textSpan of definition because it's external file
});

it('should be able to find a two-way binding', () => {
const fileName = mockHost.addCode(`
@Component({
template: '<test-comp string-model ~{start-my}[(«model»)]="test"~{end-my}></test-comp>'
})
export class MyComponent {
test = "";
}`);
// Get the marker for «model» in the code added above.
const marker = mockHost.getReferenceMarkerFor(fileName, 'model');

const result = ngService.getDefinitionAt(fileName, marker.start);
expect(result).toBeDefined();
const {textSpan, definitions} = result !;

// Get the marker for bounded text in the code added above
const boundedText = mockHost.getLocationMarkerFor(fileName, 'my');
expect(textSpan).toEqual(boundedText);

// There should be exactly 1 definition
expect(definitions).toBeDefined();
expect(definitions !.length).toBe(1);
const def = definitions ![0];

const refFileName = '/app/parsing-cases.ts';
expect(def.fileName).toBe(refFileName);
expect(def.name).toBe('model');
expect(def.kind).toBe('property');
const content = mockHost.readFile(refFileName) !;
const ref = `@Input() model: string = 'model';`;
expect(def.textSpan).toEqual({
start: content.indexOf(ref),
length: ref.length,
});
});

it('should be able to find a template from a url', () => {
const fileName = mockHost.addCode(`
@Component({
Expand Down
16 changes: 16 additions & 0 deletions packages/language-service/test/hover_spec.ts
Expand Up @@ -162,6 +162,22 @@ describe('hover', () => {
expect(toText(displayParts)).toBe('(property) NgIf<T>.ngIf: T');
});

it('should be able to find a reference to a two-way binding', () => {
const fileName = mockHost.addCode(`
@Component({
template: '<test-comp string-model «[(ᐱmodelᐱ)]="test"»></test-comp>'
})
export class MyComponent {
test = "";
}`);
const marker = mockHost.getDefinitionMarkerFor(fileName, 'model');
const quickInfo = ngLS.getHoverAt(fileName, marker.start);
expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(property) StringModel.model: string');
});

it('should be able to ignore a reference declaration', () => {
const fileName = mockHost.addCode(`
@Component({
Expand Down

2 comments on commit eb5c20c

@IgorMinar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivanwonder great contribution! thank you! do you think you could author more descriptive commit message in the future? thank you!

@ivanwonder
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IgorMinar Sorry for that. I will give a more descriptive message as I can in the next time.

Please sign in to comment.