Skip to content

Commit 7eccbcd

Browse files
Keen Yee Liaumhevery
authored andcommitted
fix(language-service): Make missing module suggestion instead of error (angular#34115)
If a Component or Directive is not part of any NgModule, the language service currently produces an error message. This should not be an error. Instead, it should be a suggestion. This PR removes `ng.DiagnosticKind`, and instead reuses `ts.DiagnosticCategory`. PR closes angular/vscode-ng-language-service#458 PR Close angular#34115
1 parent 99320e1 commit 7eccbcd

File tree

5 files changed

+22
-41
lines changed

5 files changed

+22
-41
lines changed

packages/language-service/src/diagnostics.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function getTemplateDiagnostics(ast: AstResult): ng.Diagnostic[] {
2626
if (parseErrors && parseErrors.length) {
2727
return parseErrors.map(e => {
2828
return {
29-
kind: ng.DiagnosticKind.Error,
29+
kind: ts.DiagnosticCategory.Error,
3030
span: offsetSpan(spanOf(e.span), template.span.start),
3131
message: e.msg,
3232
};
@@ -91,7 +91,7 @@ export function getDeclarationDiagnostics(
9191

9292
for (const error of errors) {
9393
results.push({
94-
kind: ng.DiagnosticKind.Error,
94+
kind: ts.DiagnosticCategory.Error,
9595
message: error.message,
9696
span: error.span,
9797
});
@@ -102,22 +102,22 @@ export function getDeclarationDiagnostics(
102102
if (metadata.isComponent) {
103103
if (!modules.ngModuleByPipeOrDirective.has(declaration.type)) {
104104
results.push({
105-
kind: ng.DiagnosticKind.Error,
105+
kind: ts.DiagnosticCategory.Suggestion,
106106
message: missingDirective(type.name, metadata.isComponent),
107107
span: declarationSpan,
108108
});
109109
}
110110
const {template, templateUrl, styleUrls} = metadata.template !;
111111
if (template === null && !templateUrl) {
112112
results.push({
113-
kind: ng.DiagnosticKind.Error,
113+
kind: ts.DiagnosticCategory.Error,
114114
message: `Component '${type.name}' must have a template or templateUrl`,
115115
span: declarationSpan,
116116
});
117117
} else if (templateUrl) {
118118
if (template) {
119119
results.push({
120-
kind: ng.DiagnosticKind.Error,
120+
kind: ts.DiagnosticCategory.Error,
121121
message: `Component '${type.name}' must not have both template and templateUrl`,
122122
span: declarationSpan,
123123
});
@@ -152,7 +152,7 @@ export function getDeclarationDiagnostics(
152152
}
153153
} else if (!directives.has(declaration.type)) {
154154
results.push({
155-
kind: ng.DiagnosticKind.Error,
155+
kind: ts.DiagnosticCategory.Suggestion,
156156
message: missingDirective(type.name, metadata.isComponent),
157157
span: declarationSpan,
158158
});
@@ -192,7 +192,7 @@ function validateUrls(
192192
if (tsLsHost.fileExists(url)) continue;
193193

194194
allErrors.push({
195-
kind: ng.DiagnosticKind.Error,
195+
kind: ts.DiagnosticCategory.Error,
196196
message: `URL does not point to a valid file`,
197197
// Exclude opening and closing quotes in the url span.
198198
span: {start: urlNode.getStart() + 1, end: urlNode.end - 1},
@@ -226,7 +226,7 @@ export function ngDiagnosticToTsDiagnostic(
226226
start: d.span.start,
227227
length: d.span.end - d.span.start,
228228
messageText: typeof d.message === 'string' ? d.message : chainDiagnostics(d.message),
229-
category: ts.DiagnosticCategory.Error,
229+
category: d.kind,
230230
code: 0,
231231
source: 'ng',
232232
};

packages/language-service/src/expression_diagnostics.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
*/
88

99
import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, CompileDirectiveSummary, CompileTypeMetadata, DirectiveAst, ElementAst, EmbeddedTemplateAst, Node, ParseSourceSpan, RecursiveTemplateAstVisitor, ReferenceAst, TemplateAst, TemplateAstPath, VariableAst, findNode, identifierName, templateVisitAll, tokenReference} from '@angular/compiler';
10+
import * as ts from 'typescript';
1011

11-
import {AstType, DiagnosticKind, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type';
12+
import {AstType, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type';
1213
import {BuiltinType, Definition, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols';
14+
import {Diagnostic} from './types';
1315

1416
export interface DiagnosticTemplateInfo {
1517
fileName?: string;
@@ -20,14 +22,7 @@ export interface DiagnosticTemplateInfo {
2022
templateAst: TemplateAst[];
2123
}
2224

23-
export interface ExpressionDiagnostic {
24-
message: string;
25-
span: Span;
26-
kind: DiagnosticKind;
27-
}
28-
29-
export function getTemplateExpressionDiagnostics(info: DiagnosticTemplateInfo):
30-
ExpressionDiagnostic[] {
25+
export function getTemplateExpressionDiagnostics(info: DiagnosticTemplateInfo): Diagnostic[] {
3126
const visitor = new ExpressionDiagnosticsVisitor(
3227
info, (path: TemplateAstPath, includeEvent: boolean) =>
3328
getExpressionScope(info, path, includeEvent));
@@ -228,7 +223,7 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
228223
// TODO(issue/24571): remove '!'.
229224
private directiveSummary !: CompileDirectiveSummary;
230225

231-
diagnostics: ExpressionDiagnostic[] = [];
226+
diagnostics: Diagnostic[] = [];
232227

233228
constructor(
234229
private info: DiagnosticTemplateInfo,
@@ -335,13 +330,13 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
335330
private reportError(message: string, span: Span|undefined) {
336331
if (span) {
337332
this.diagnostics.push(
338-
{span: offsetSpan(span, this.info.offset), kind: DiagnosticKind.Error, message});
333+
{span: offsetSpan(span, this.info.offset), kind: ts.DiagnosticCategory.Error, message});
339334
}
340335
}
341336

342337
private reportWarning(message: string, span: Span) {
343338
this.diagnostics.push(
344-
{span: offsetSpan(span, this.info.offset), kind: DiagnosticKind.Warning, message});
339+
{span: offsetSpan(span, this.info.offset), kind: ts.DiagnosticCategory.Warning, message});
345340
}
346341
}
347342

packages/language-service/src/expression_type.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,14 @@
77
*/
88

99
import {AST, AstVisitor, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, visitAstChildren} from '@angular/compiler';
10+
import * as ts from 'typescript';
1011

11-
import {BuiltinType, Signature, Span, Symbol, SymbolQuery, SymbolTable} from './symbols';
12+
import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols';
1213

1314
export interface ExpressionDiagnosticsContext { event?: boolean; }
1415

15-
export enum DiagnosticKind {
16-
Error,
17-
Warning,
18-
}
19-
2016
export class TypeDiagnostic {
21-
constructor(public kind: DiagnosticKind, public message: string, public ast: AST) {}
17+
constructor(public kind: ts.DiagnosticCategory, public message: string, public ast: AST) {}
2218
}
2319

2420
// AstType calculatetype of the ast given AST element.
@@ -412,14 +408,14 @@ export class AstType implements AstVisitor {
412408

413409
private reportError(message: string, ast: AST): Symbol {
414410
if (this.diagnostics) {
415-
this.diagnostics.push(new TypeDiagnostic(DiagnosticKind.Error, message, ast));
411+
this.diagnostics.push(new TypeDiagnostic(ts.DiagnosticCategory.Error, message, ast));
416412
}
417413
return this.anyType;
418414
}
419415

420416
private reportWarning(message: string, ast: AST): Symbol {
421417
if (this.diagnostics) {
422-
this.diagnostics.push(new TypeDiagnostic(DiagnosticKind.Warning, message, ast));
418+
this.diagnostics.push(new TypeDiagnostic(ts.DiagnosticCategory.Warning, message, ast));
423419
}
424420
return this.anyType;
425421
}

packages/language-service/src/types.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,16 +240,6 @@ export interface Location {
240240
span: Span;
241241
}
242242

243-
/**
244-
* The kind of diagnostic message.
245-
*
246-
* @publicApi
247-
*/
248-
export enum DiagnosticKind {
249-
Error,
250-
Warning,
251-
}
252-
253243
/**
254244
* The type of Angular directive. Used for QuickInfo in template.
255245
*/
@@ -314,7 +304,7 @@ export interface Diagnostic {
314304
/**
315305
* The kind of diagnostic message
316306
*/
317-
kind: DiagnosticKind;
307+
kind: ts.DiagnosticCategory;
318308

319309
/**
320310
* The source span that should be highlighted.

packages/language-service/src/typescript_host.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {AstResult} from './common';
1515
import {createLanguageService} from './language_service';
1616
import {ReflectorHost} from './reflector_host';
1717
import {ExternalTemplate, InlineTemplate, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './template';
18-
import {Declaration, DeclarationError, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
18+
import {Declaration, DeclarationError, Diagnostic, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
1919
import {findTightestNode, getDirectiveClassLike} from './utils';
2020

2121

0 commit comments

Comments
 (0)