Skip to content
Permalink
Browse files

fix(language-service): Make missing module suggestion instead of error (

#34115) (#34193)

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 #34115

(cherry picked from commit 7eccbcd)

PR Close #34193
  • Loading branch information
kyliau authored and mhevery committed Nov 28, 2019
1 parent dd6da21 commit d2538cab5a3d547ea606016afc1a7d50b692e0a1
@@ -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,22 +102,22 @@ 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,
});
}
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',
};
@@ -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});
}
}

@@ -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;
}
@@ -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.
@@ -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';


0 comments on commit d2538ca

Please sign in to comment.
You can’t perform that action at this time.