fix(language-service): Make missing module suggestion instead of error (#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 https://github.com/angular/vscode-ng-language-service/issues/458

PR Close #34115
This commit is contained in:
Keen Yee Liau 2019-11-27 16:31:00 -08:00 committed by Miško Hevery
parent 99320e1ffc
commit 7eccbcd30d
5 changed files with 22 additions and 41 deletions

View File

@ -26,7 +26,7 @@ export function getTemplateDiagnostics(ast: AstResult): ng.Diagnostic[] {
if (parseErrors && parseErrors.length) {
return parseErrors.map(e => {
return {
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Error,
span: offsetSpan(spanOf(e.span), template.span.start),
message: e.msg,
};
@ -91,7 +91,7 @@ export function getDeclarationDiagnostics(
for (const error of errors) {
results.push({
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Error,
message: error.message,
span: error.span,
});
@ -102,7 +102,7 @@ export function getDeclarationDiagnostics(
if (metadata.isComponent) {
if (!modules.ngModuleByPipeOrDirective.has(declaration.type)) {
results.push({
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Suggestion,
message: missingDirective(type.name, metadata.isComponent),
span: declarationSpan,
});
@ -110,14 +110,14 @@ export function getDeclarationDiagnostics(
const {template, templateUrl, styleUrls} = metadata.template !;
if (template === null && !templateUrl) {
results.push({
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Error,
message: `Component '${type.name}' must have a template or templateUrl`,
span: declarationSpan,
});
} else if (templateUrl) {
if (template) {
results.push({
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Error,
message: `Component '${type.name}' must not have both template and templateUrl`,
span: declarationSpan,
});
@ -152,7 +152,7 @@ export function getDeclarationDiagnostics(
}
} else if (!directives.has(declaration.type)) {
results.push({
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Suggestion,
message: missingDirective(type.name, metadata.isComponent),
span: declarationSpan,
});
@ -192,7 +192,7 @@ function validateUrls(
if (tsLsHost.fileExists(url)) continue;
allErrors.push({
kind: ng.DiagnosticKind.Error,
kind: ts.DiagnosticCategory.Error,
message: `URL does not point to a valid file`,
// Exclude opening and closing quotes in the url span.
span: {start: urlNode.getStart() + 1, end: urlNode.end - 1},
@ -226,7 +226,7 @@ export function ngDiagnosticToTsDiagnostic(
start: d.span.start,
length: d.span.end - d.span.start,
messageText: typeof d.message === 'string' ? d.message : chainDiagnostics(d.message),
category: ts.DiagnosticCategory.Error,
category: d.kind,
code: 0,
source: 'ng',
};

View File

@ -7,9 +7,11 @@
*/
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';
import * as ts from 'typescript';
import {AstType, DiagnosticKind, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type';
import {AstType, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type';
import {BuiltinType, Definition, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols';
import {Diagnostic} from './types';
export interface DiagnosticTemplateInfo {
fileName?: string;
@ -20,14 +22,7 @@ export interface DiagnosticTemplateInfo {
templateAst: TemplateAst[];
}
export interface ExpressionDiagnostic {
message: string;
span: Span;
kind: DiagnosticKind;
}
export function getTemplateExpressionDiagnostics(info: DiagnosticTemplateInfo):
ExpressionDiagnostic[] {
export function getTemplateExpressionDiagnostics(info: DiagnosticTemplateInfo): Diagnostic[] {
const visitor = new ExpressionDiagnosticsVisitor(
info, (path: TemplateAstPath, includeEvent: boolean) =>
getExpressionScope(info, path, includeEvent));
@ -228,7 +223,7 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
// TODO(issue/24571): remove '!'.
private directiveSummary !: CompileDirectiveSummary;
diagnostics: ExpressionDiagnostic[] = [];
diagnostics: Diagnostic[] = [];
constructor(
private info: DiagnosticTemplateInfo,
@ -335,13 +330,13 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
private reportError(message: string, span: Span|undefined) {
if (span) {
this.diagnostics.push(
{span: offsetSpan(span, this.info.offset), kind: DiagnosticKind.Error, message});
{span: offsetSpan(span, this.info.offset), kind: ts.DiagnosticCategory.Error, message});
}
}
private reportWarning(message: string, span: Span) {
this.diagnostics.push(
{span: offsetSpan(span, this.info.offset), kind: DiagnosticKind.Warning, message});
{span: offsetSpan(span, this.info.offset), kind: ts.DiagnosticCategory.Warning, message});
}
}

View File

@ -7,18 +7,14 @@
*/
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';
import * as ts from 'typescript';
import {BuiltinType, Signature, Span, Symbol, SymbolQuery, SymbolTable} from './symbols';
import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols';
export interface ExpressionDiagnosticsContext { event?: boolean; }
export enum DiagnosticKind {
Error,
Warning,
}
export class TypeDiagnostic {
constructor(public kind: DiagnosticKind, public message: string, public ast: AST) {}
constructor(public kind: ts.DiagnosticCategory, public message: string, public ast: AST) {}
}
// AstType calculatetype of the ast given AST element.
@ -412,14 +408,14 @@ export class AstType implements AstVisitor {
private reportError(message: string, ast: AST): Symbol {
if (this.diagnostics) {
this.diagnostics.push(new TypeDiagnostic(DiagnosticKind.Error, message, ast));
this.diagnostics.push(new TypeDiagnostic(ts.DiagnosticCategory.Error, message, ast));
}
return this.anyType;
}
private reportWarning(message: string, ast: AST): Symbol {
if (this.diagnostics) {
this.diagnostics.push(new TypeDiagnostic(DiagnosticKind.Warning, message, ast));
this.diagnostics.push(new TypeDiagnostic(ts.DiagnosticCategory.Warning, message, ast));
}
return this.anyType;
}

View File

@ -240,16 +240,6 @@ export interface Location {
span: Span;
}
/**
* The kind of diagnostic message.
*
* @publicApi
*/
export enum DiagnosticKind {
Error,
Warning,
}
/**
* The type of Angular directive. Used for QuickInfo in template.
*/
@ -314,7 +304,7 @@ export interface Diagnostic {
/**
* The kind of diagnostic message
*/
kind: DiagnosticKind;
kind: ts.DiagnosticCategory;
/**
* The source span that should be highlighted.

View File

@ -15,7 +15,7 @@ import {AstResult} from './common';
import {createLanguageService} from './language_service';
import {ReflectorHost} from './reflector_host';
import {ExternalTemplate, InlineTemplate, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './template';
import {Declaration, DeclarationError, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
import {Declaration, DeclarationError, Diagnostic, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
import {findTightestNode, getDirectiveClassLike} from './utils';