From 84ba1f012ea3f019b45dacdd898076b817a573c7 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Fri, 11 Oct 2019 13:56:33 -0700 Subject: [PATCH] test(language-service): Remove redundant marker methods in MockHost (#33115) Remove the following methods from MockHost: 1. `getMarkerLocations`: Replaced with `getLocationMarkerFor()` 2. `getReferenceMarkers`: Replaced with `getReferenceMarkerFor()` PR Close #33115 --- .../language-service/test/completions_spec.ts | 7 +- .../test/language_service_spec.ts | 2 +- .../test/project/app/expression-cases.ts | 8 +- .../test/project/app/ng-for-cases.ts | 6 +- .../test/project/app/ng-if-cases.ts | 2 +- packages/language-service/test/test_utils.ts | 107 +++++++++++------- .../language-service/test/ts_plugin_spec.ts | 30 ++--- 7 files changed, 85 insertions(+), 77 deletions(-) diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index b1163d93a3..b97f374099 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -196,11 +196,8 @@ describe('completions', () => { }); function expectContains(fileName: string, locationMarker: string, ...names: string[]) { - let location = mockHost.getMarkerLocations(fileName) ![locationMarker]; - if (location == null) { - throw new Error(`No marker ${locationMarker} found.`); - } - expectEntries(locationMarker, ngService.getCompletionsAt(fileName, location), ...names); + const marker = mockHost.getLocationMarkerFor(fileName, locationMarker); + expectEntries(locationMarker, ngService.getCompletionsAt(fileName, marker.start), ...names); } }); diff --git a/packages/language-service/test/language_service_spec.ts b/packages/language-service/test/language_service_spec.ts index 96e01dfee0..1898a22038 100644 --- a/packages/language-service/test/language_service_spec.ts +++ b/packages/language-service/test/language_service_spec.ts @@ -20,7 +20,7 @@ describe('service without angular', () => { let ngHost = new TypeScriptServiceHost(mockHost, service); let ngService = createLanguageService(ngHost); const fileName = '/app/test.ng'; - let position = mockHost.getMarkerLocations(fileName) !['h1-content']; + const position = mockHost.getLocationMarkerFor(fileName, 'h1-content').start; it('should not crash a get template references', () => expect(() => ngService.getTemplateReferences())); diff --git a/packages/language-service/test/project/app/expression-cases.ts b/packages/language-service/test/project/app/expression-cases.ts index 84a78f2abb..decc717a62 100644 --- a/packages/language-service/test/project/app/expression-cases.ts +++ b/packages/language-service/test/project/app/expression-cases.ts @@ -14,28 +14,28 @@ export interface Person { } @Component({ - template: '{{~{foo}foo~{foo-end}}}', + template: '{{~{start-foo}foo~{end-foo}}}', }) export class WrongFieldReference { bar = 'bar'; } @Component({ - template: '{{~{nam}person.nam~{nam-end}}}', + template: '{{~{start-nam}person.nam~{end-nam}}}', }) export class WrongSubFieldReference { person: Person = {name: 'Bob', age: 23}; } @Component({ - template: '{{~{myField}myField~{myField-end}}}', + template: '{{~{start-myField}myField~{end-myField}}}', }) export class PrivateReference { private myField = 'My Field'; } @Component({ - template: '{{~{mod}"a" ~{mod-end}% 2}}', + template: '{{~{start-mod}"a" ~{end-mod}% 2}}', }) export class ExpectNumericType { } diff --git a/packages/language-service/test/project/app/ng-for-cases.ts b/packages/language-service/test/project/app/ng-for-cases.ts index 806daf5730..9b46ed5dc3 100644 --- a/packages/language-service/test/project/app/ng-for-cases.ts +++ b/packages/language-service/test/project/app/ng-for-cases.ts @@ -15,7 +15,7 @@ export interface Person { @Component({ template: ` -
+
{{person.name}}
`, }) @@ -24,7 +24,7 @@ export class UnknownPeople { @Component({ template: ` -
+
{{person.name}}
`, }) @@ -34,7 +34,7 @@ export class UnknownEven { @Component({ template: ` -
+
{{person.name}}
`, }) diff --git a/packages/language-service/test/project/app/ng-if-cases.ts b/packages/language-service/test/project/app/ng-if-cases.ts index 9d556235dc..3c9917d772 100644 --- a/packages/language-service/test/project/app/ng-if-cases.ts +++ b/packages/language-service/test/project/app/ng-if-cases.ts @@ -10,7 +10,7 @@ import {Component} from '@angular/core'; @Component({ template: ` -
+
Showing now!
`, }) diff --git a/packages/language-service/test/test_utils.ts b/packages/language-service/test/test_utils.ts index 4bf5074054..82769bd195 100644 --- a/packages/language-service/test/test_utils.ts +++ b/packages/language-service/test/test_utils.ts @@ -181,20 +181,6 @@ export class MockTypescriptHost implements ts.LanguageServiceHost { } } - getMarkerLocations(fileName: string): {[name: string]: number}|undefined { - let content = this.getRawFileContent(fileName); - if (content) { - return getLocationMarkers(content); - } - } - - getReferenceMarkers(fileName: string): ReferenceResult|undefined { - let content = this.getRawFileContent(fileName); - if (content) { - return getReferenceMarkers(content); - } - } - /** * Reset the project to its original state, effectively removing all overrides. */ @@ -279,61 +265,94 @@ export class MockTypescriptHost implements ts.LanguageServiceHost { } /** - * Returns the definition marker ᐱselectorᐱ for the specified 'selector'. + * Returns the definition marker `ᐱselectorᐱ` for the specified 'selector'. * Asserts that marker exists. * @param fileName name of the file * @param selector name of the marker */ getDefinitionMarkerFor(fileName: string, selector: string): ts.TextSpan { - const markers = this.getReferenceMarkers(fileName); - expect(markers).toBeDefined(); - expect(Object.keys(markers !.definitions)).toContain(selector); - expect(markers !.definitions[selector].length).toBe(1); - const marker = markers !.definitions[selector][0]; - expect(marker.start).toBeLessThanOrEqual(marker.end); + const content = this.getRawFileContent(fileName); + if (!content) { + throw new Error(`File does not exist: ${fileName}`); + } + const markers = getReferenceMarkers(content); + const definitions = markers.definitions[selector]; + if (!definitions || !definitions.length) { + throw new Error(`Failed to find marker '${selector}' in ${fileName}`); + } + if (definitions.length > 1) { + throw new Error(`Multiple positions found for '${selector}' in ${fileName}`); + } + const {start, end} = definitions[0]; + if (start > end) { + throw new Error(`Marker '${selector}' in ${fileName} is invalid: ${start} > ${end}`); + } return { - start: marker.start, - length: marker.end - marker.start, + start, + length: end - start, }; } /** - * Returns the reference marker «selector» for the specified 'selector'. + * Returns the reference marker `«selector»` for the specified 'selector'. * Asserts that marker exists. * @param fileName name of the file * @param selector name of the marker */ getReferenceMarkerFor(fileName: string, selector: string): ts.TextSpan { - const markers = this.getReferenceMarkers(fileName); - expect(markers).toBeDefined(); - expect(Object.keys(markers !.references)).toContain(selector); - expect(markers !.references[selector].length).toBe(1); - const marker = markers !.references[selector][0]; - expect(marker.start).toBeLessThanOrEqual(marker.end); + const content = this.getRawFileContent(fileName); + if (!content) { + throw new Error(`File does not exist: ${fileName}`); + } + const markers = getReferenceMarkers(content); + const references = markers.references[selector]; + if (!references || !references.length) { + throw new Error(`Failed to find marker '${selector}' in ${fileName}`); + } + if (references.length > 1) { + throw new Error(`Multiple positions found for '${selector}' in ${fileName}`); + } + const {start, end} = references[0]; + if (start > end) { + throw new Error(`Marker '${selector}' in ${fileName} is invalid: ${start} > ${end}`); + } return { - start: marker.start, - length: marker.end - marker.start, + start, + length: end - start, }; } /** - * Returns the location marker ~{selector} for the specified 'selector'. + * Returns the location marker `~{selector}` or the marker pair + * `~{start-selector}` and `~{end-selector}` for the specified 'selector'. * Asserts that marker exists. * @param fileName name of the file * @param selector name of the marker */ getLocationMarkerFor(fileName: string, selector: string): ts.TextSpan { - const markers = this.getMarkerLocations(fileName); - expect(markers).toBeDefined(); - const start = markers ![`start-${selector}`]; - expect(start).toBeDefined(); - const end = markers ![`end-${selector}`]; - expect(end).toBeDefined(); - expect(start).toBeLessThanOrEqual(end); - return { - start: start, - length: end - start, - }; + const content = this.getRawFileContent(fileName); + if (!content) { + throw new Error(`File does not exist: ${fileName}`); + } + const markers = getLocationMarkers(content); + // Look for just the selector itself + const position = markers[selector]; + if (position !== undefined) { + return { + start: position, + length: 0, + }; + } + // Look for start and end markers for the selector + const start = markers[`start-${selector}`]; + const end = markers[`end-${selector}`]; + if (start !== undefined && end !== undefined) { + return { + start, + length: end - start, + }; + } + throw new Error(`Failed to find marker '${selector}' in ${fileName}`); } } diff --git a/packages/language-service/test/ts_plugin_spec.ts b/packages/language-service/test/ts_plugin_spec.ts index b7a49bea79..781db1a9fa 100644 --- a/packages/language-service/test/ts_plugin_spec.ts +++ b/packages/language-service/test/ts_plugin_spec.ts @@ -167,7 +167,7 @@ describe('plugin', () => { 'Identifier \'people_1\' is not defined. The component declaration, template variable declarations, and element references do not contain such a member'); }); it('should report an unknown context reference', () => { - expectError('even_1', 'The template context does not define a member called \'even_1\''); + expectError('even_1', `The template context does not define a member called 'even_1'`); }); it('should report an unknown value in a key expression', () => { expectError( @@ -180,7 +180,7 @@ describe('plugin', () => { expectSemanticError('/app/ng-if-cases.ts', locationMarker, message); } it('should report an implicit context reference', () => { - expectError('implicit', 'The template context does not define a member called \'unknown\''); + expectError('implicit', `The template context does not define a member called 'unknown'`); }); }); @@ -199,11 +199,10 @@ describe('plugin', () => { it('should be able to get entity completions', () => { const fileName = '/app/app.component.ts'; - const marker = 'entity-amp'; - const position = getMarkerLocation(fileName, marker); - const results = ngLS.getCompletionsAtPosition(fileName, position, {} /* options */); + const marker = mockHost.getLocationMarkerFor(fileName, 'entity-amp'); + const results = ngLS.getCompletionsAtPosition(fileName, marker.start, {} /* options */); expect(results).toBeTruthy(); - expectEntries(marker, results !, ...['&', '>', '<', 'ι']); + expectEntries('entity-amp', results !, ...['&', '>', '<', 'ι']); }); it('should report template diagnostics', () => { @@ -231,27 +230,20 @@ describe('plugin', () => { }); } - function getMarkerLocation(fileName: string, locationMarker: string): number { - const location = mockHost.getMarkerLocations(fileName) ![locationMarker]; - if (location == null) { - throw new Error(`No marker ${locationMarker} found.`); - } - return location; - } function contains(fileName: string, locationMarker: string, ...names: string[]) { - const location = getMarkerLocation(fileName, locationMarker); + const marker = mockHost.getLocationMarkerFor(fileName, locationMarker); expectEntries( - locationMarker, plugin.getCompletionsAtPosition(fileName, location, undefined) !, ...names); + locationMarker, plugin.getCompletionsAtPosition(fileName, marker.start, undefined) !, + ...names); } function expectSemanticError(fileName: string, locationMarker: string, message: string) { - const start = getMarkerLocation(fileName, locationMarker); - const end = getMarkerLocation(fileName, locationMarker + '-end'); + const marker = mockHost.getLocationMarkerFor(fileName, locationMarker); const errors = plugin.getSemanticDiagnostics(fileName); for (const error of errors) { if (error.messageText.toString().indexOf(message) >= 0) { - expect(error.start).toEqual(start); - expect(error.length).toEqual(end - start); + expect(error.start).toEqual(marker.start); + expect(error.length).toEqual(marker.length); return; } }