From 8bead6bfdda908dfc4efd194a57c77009a2a7b6e Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Fri, 5 Jun 2020 22:42:35 -0700 Subject: [PATCH] test(language-service): Remove all markers from test project (#37475) This commit removes all markers from the inline template in `AppComponent` and external template in `TemplateReference`. Test scenarios should be colocated with the test cases themselves. Besides, many existing cases are invalid. For example, if we want to test autocomplete for HTML element, the existing test case is like: ``` <~{cursor} h1> ``` This doesn't make much sense, becasue the language service already sees the `h1` tag in the template. The correct test case should be: ``` <~{cursor ``` IMO, this reflects the real-world use case better. This commit also uncovers a bug in the way HTML entities autocompletion is done. There's an off-by-one error in which a cursor that immediately trails the ampersand character fails to trigger HTML entities autocompletion. PR Close #37475 --- packages/language-service/src/completions.ts | 8 +- .../language-service/test/completions_spec.ts | 74 ++++++++++--------- .../test/language_service_spec.ts | 13 ++-- .../test/project/app/app.component.ts | 13 +--- .../language-service/test/project/app/test.ng | 9 +-- 5 files changed, 60 insertions(+), 57 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index a69571dee7..4225fbaa18 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -361,12 +361,18 @@ function elementCompletions(info: ng.AstResult): ng.CompletionEntry[] { function entityCompletions(value: string, position: number): ng.CompletionEntry[] { // Look for entity completions + // TODO(kyliau): revisit the usefulness of this feature. It provides + // autocompletion for HTML entities, which IMO is outside the core functionality + // of Angular language service. Besides, we do not have a complete list. + // See https://dev.w3.org/html5/html-author/charref const re = /&[A-Za-z]*;?(?!\d)/g; let found: RegExpExecArray|null; let result: ng.CompletionEntry[] = []; while (found = re.exec(value)) { let len = found[0].length; - if (position >= found.index && position < (found.index + len)) { + // end position must be inclusive to account for cases like '&|' where + // cursor is right behind the ampersand. + if (position >= found.index && position <= (found.index + len)) { result = Object.keys(NAMED_ENTITIES).map(name => { return { name: `&${name};`, diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index cf12cae2ff..63af0e4e07 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -15,9 +15,7 @@ import {TypeScriptServiceHost} from '../src/typescript_host'; import {MockTypescriptHost} from './test_utils'; const APP_COMPONENT = '/app/app.component.ts'; -const PARSING_CASES = '/app/parsing-cases.ts'; const TEST_TEMPLATE = '/app/test.ng'; -const EXPRESSION_CASES = '/app/expression-cases.ts'; describe('completions', () => { const mockHost = new MockTypescriptHost(['/app/main.ts']); @@ -30,22 +28,22 @@ describe('completions', () => { }); it('should be able to get entity completions', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'entity-amp'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '&~{cursor}'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.ENTITY, ['&', '>', '<', 'ι']); }); it('should be able to return html elements', () => { - const locations = ['empty', 'start-tag-h1', 'h1-content', 'start-tag', 'start-tag-after-h']; - for (const location of locations) { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, location); - const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); - expectContain(completions, CompletionKind.HTML_ELEMENT, ['div', 'h1', 'h2', 'span']); - } + mockHost.overrideInlineTemplate(APP_COMPONENT, '<~{cursor}'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); + const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); + expectContain(completions, CompletionKind.HTML_ELEMENT, ['div', 'h1', 'h2', 'span']); }); it('should be able to return component directives', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'empty'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '<~{cursor}'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.COMPONENT, [ 'ng-form', @@ -56,13 +54,15 @@ describe('completions', () => { }); it('should be able to return attribute directives', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'h1-after-space'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '

'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.ATTRIBUTE, ['string-model', 'number-model']); }); it('should be able to return angular pseudo elements', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'empty'); + mockHost.overrideInlineTemplate(APP_COMPONENT, `<~{cursor}`); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.ANGULAR_ELEMENT, [ 'ng-container', @@ -72,7 +72,8 @@ describe('completions', () => { }); it('should be able to return h1 attributes', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'h1-after-space'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '

'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.HTML_ATTRIBUTE, [ 'class', @@ -83,7 +84,8 @@ describe('completions', () => { }); it('should be able to find common Angular attributes', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'div-attributes'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '
'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.ATTRIBUTE, [ 'ngClass', @@ -95,13 +97,15 @@ describe('completions', () => { }); it('should be able to get the completions at the beginning of an interpolation', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'h2-hero'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '

{{ ~{cursor} }}

'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.PROPERTY, ['title', 'hero']); }); it('should not include private members of a class', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'h2-hero'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '

{{ ~{cursor} }}

'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expect(completions).toBeDefined(); const internal = completions!.entries.find(e => e.name === 'internal'); @@ -109,13 +113,15 @@ describe('completions', () => { }); it('should be able to get the completions at the end of an interpolation', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'sub-end'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '{{ti~{cursor}}}'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.PROPERTY, ['title', 'hero']); }); it('should be able to get the completions in a property', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'h2-name'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '

{{ hero.~{cursor} }}

'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']); }); @@ -184,7 +190,8 @@ describe('completions', () => { }); it('should suggest $any() type cast function in an interpolation', () => { - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'sub-start'); + mockHost.overrideInlineTemplate(APP_COMPONENT, '{{ ~{cursor} }}'); + const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); expectContain(completions, CompletionKind.METHOD, ['$any']); }); @@ -235,27 +242,27 @@ describe('completions', () => { describe('in external template', () => { it('should be able to get entity completions in external template', () => { - const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'entity-amp'); + mockHost.override(TEST_TEMPLATE, '&~{cursor}'); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); expectContain(completions, CompletionKind.ENTITY, ['&', '>', '<', 'ι']); }); it('should not return html elements', () => { - const locations = ['empty', 'start-tag-h1', 'h1-content', 'start-tag', 'start-tag-after-h']; - for (const location of locations) { - const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, location); - const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); - expect(completions).toBeDefined(); - const {entries} = completions!; - expect(entries).not.toContain(jasmine.objectContaining({name: 'div'})); - expect(entries).not.toContain(jasmine.objectContaining({name: 'h1'})); - expect(entries).not.toContain(jasmine.objectContaining({name: 'h2'})); - expect(entries).not.toContain(jasmine.objectContaining({name: 'span'})); - } + mockHost.override(TEST_TEMPLATE, '<~{cursor}'); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); + const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); + expect(completions).toBeDefined(); + const {entries} = completions!; + expect(entries).not.toContain(jasmine.objectContaining({name: 'div'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'h1'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'h2'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'span'})); }); it('should be able to return element directives', () => { - const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'empty'); + mockHost.override(TEST_TEMPLATE, '<~{cursor}'); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); expectContain(completions, CompletionKind.COMPONENT, [ 'ng-form', @@ -266,7 +273,8 @@ describe('completions', () => { }); it('should not return html attributes', () => { - const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'h1-after-space'); + mockHost.override(TEST_TEMPLATE, '

'); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); expect(completions).toBeDefined(); const {entries} = completions!; diff --git a/packages/language-service/test/language_service_spec.ts b/packages/language-service/test/language_service_spec.ts index b9f9c5f031..46742f7cce 100644 --- a/packages/language-service/test/language_service_spec.ts +++ b/packages/language-service/test/language_service_spec.ts @@ -18,27 +18,28 @@ describe('service without angular', () => { const service = ts.createLanguageService(mockHost); const ngHost = new TypeScriptServiceHost(mockHost, service); const ngService = createLanguageService(ngHost); - const fileName = '/app/test.ng'; - const position = mockHost.getLocationMarkerFor(fileName, 'h1-content').start; + const TEST_TEMPLATE = '/app/test.ng'; + mockHost.override(TEST_TEMPLATE, '

~{cursor}

'); + const position = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor').start; beforeEach(() => { mockHost.reset(); }); it('should not crash a get diagnostics', () => { - expect(() => ngService.getSemanticDiagnostics(fileName)).not.toThrow(); + expect(() => ngService.getSemanticDiagnostics(TEST_TEMPLATE)).not.toThrow(); }); it('should not crash a completion', () => { - expect(() => ngService.getCompletionsAtPosition(fileName, position)).not.toThrow(); + expect(() => ngService.getCompletionsAtPosition(TEST_TEMPLATE, position)).not.toThrow(); }); it('should not crash a get definition', () => { - expect(() => ngService.getDefinitionAndBoundSpan(fileName, position)).not.toThrow(); + expect(() => ngService.getDefinitionAndBoundSpan(TEST_TEMPLATE, position)).not.toThrow(); }); it('should not crash a hover', () => { - expect(() => ngService.getQuickInfoAtPosition(fileName, position)).not.toThrow(); + expect(() => ngService.getQuickInfoAtPosition(TEST_TEMPLATE, position)).not.toThrow(); }); it('should not crash with an incomplete class', () => { diff --git a/packages/language-service/test/project/app/app.component.ts b/packages/language-service/test/project/app/app.component.ts index ad833ef051..768b8d11e7 100644 --- a/packages/language-service/test/project/app/app.component.ts +++ b/packages/language-service/test/project/app/app.component.ts @@ -15,16 +15,9 @@ export interface Hero { @Component({ selector: 'my-app', - template: `~{empty} - <~{start-tag}h~{start-tag-after-h}1~{start-tag-h1} ~{h1-after-space}> - ~{h1-content} {{~{sub-start}title~{sub-end}}} -

- ~{after-h1}

{{~{h2-hero}hero.~{h2-name}name}} details!

-
{{~{label-hero}hero.~{label-id}id}}
-
- -
- &~{entity-amp}amp; + template: ` +

{{title}}

+

{{hero.name}} details!

` }) export class AppComponent { diff --git a/packages/language-service/test/project/app/test.ng b/packages/language-service/test/project/app/test.ng index d6717b81b5..ba6fb35be3 100644 --- a/packages/language-service/test/project/app/test.ng +++ b/packages/language-service/test/project/app/test.ng @@ -1,7 +1,2 @@ -~{empty} -<~{start-tag}h~{start-tag-after-h}1~{start-tag-h1} ~{h1-after-space}> - ~{h1-content} {{~{sub-start}title~{sub-end}}} - -~{after-h1}

{{~{h2-hero}hero.~{h2-name}name}} details!

-
{{~{label-hero}hero.~{label-id}id}}
-&~{entity-amp}amp; +

{{title}}

+

{{hero.name}} details!