fix(compiler-cli): report non-template diagnostics (#40331)
Report non-template diagnotics when calling `getDiagnotics` function of the language service we only returned template diagnotics. This change causes it to return all diagnotics, not just diagnostics from the template type checker. PR Close #40331
This commit is contained in:
		
							parent
							
								
									625d2c252b
								
							
						
					
					
						commit
						4db89f4576
					
				| @ -33,6 +33,7 @@ ts_library( | ||||
|         "//packages/compiler-cli/src/ngtsc/shims", | ||||
|         "//packages/compiler-cli/src/ngtsc/translator", | ||||
|         "//packages/compiler-cli/src/ngtsc/typecheck", | ||||
|         "//packages/compiler-cli/src/ngtsc/typecheck/api", | ||||
|         "@npm//@bazel/typescript", | ||||
|         "@npm//@types/node", | ||||
|         "@npm//chokidar", | ||||
|  | ||||
| @ -30,7 +30,6 @@ import {ivySwitchTransform} from '../../switch'; | ||||
| import {aliasTransformFactory, CompilationMode, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform'; | ||||
| import {TemplateTypeCheckerImpl} from '../../typecheck'; | ||||
| import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy} from '../../typecheck/api'; | ||||
| import {isTemplateDiagnostic} from '../../typecheck/diagnostics'; | ||||
| import {getSourceFileOrNull, isDtsPath, resolveModuleName} from '../../util/src/typescript'; | ||||
| import {LazyRoute, NgCompilerAdapter, NgCompilerOptions} from '../api'; | ||||
| 
 | ||||
| @ -84,11 +83,12 @@ export class NgCompiler { | ||||
|   private constructionDiagnostics: ts.Diagnostic[] = []; | ||||
| 
 | ||||
|   /** | ||||
|    * Semantic diagnostics related to the program itself. | ||||
|    * Non-template diagnostics related to the program itself. Does not include template | ||||
|    * diagnostics because the template type checker memoizes them itself. | ||||
|    * | ||||
|    * This is set by (and memoizes) `getDiagnostics`. | ||||
|    * This is set by (and memoizes) `getNonTemplateDiagnostics`. | ||||
|    */ | ||||
|   private diagnostics: ts.Diagnostic[]|null = null; | ||||
|   private nonTemplateDiagnostics: ts.Diagnostic[]|null = null; | ||||
| 
 | ||||
|   private closureCompilerEnabled: boolean; | ||||
|   private nextProgram: ts.Program; | ||||
| @ -175,38 +175,23 @@ export class NgCompiler { | ||||
|     return this.incrementalDriver.depGraph.getResourceDependencies(file); | ||||
|   } | ||||
| 
 | ||||
|   /** | ||||
|    * Get all Angular-related diagnostics for this compilation. | ||||
|    */ | ||||
|   getDiagnostics(): ts.Diagnostic[] { | ||||
|     return [...this.getNonTemplateDiagnostics(), ...this.getTemplateDiagnostics()]; | ||||
|   } | ||||
| 
 | ||||
|   /** | ||||
|    * Get all Angular-related diagnostics for this compilation. | ||||
|    * | ||||
|    * If a `ts.SourceFile` is passed, only diagnostics related to that file are returned. | ||||
|    */ | ||||
|   getDiagnostics(file?: ts.SourceFile): ts.Diagnostic[] { | ||||
|     if (this.diagnostics === null) { | ||||
|       const compilation = this.ensureAnalyzed(); | ||||
|       this.diagnostics = | ||||
|           [...compilation.traitCompiler.diagnostics, ...this.getTemplateDiagnostics()]; | ||||
|       if (this.entryPoint !== null && compilation.exportReferenceGraph !== null) { | ||||
|         this.diagnostics.push(...checkForPrivateExports( | ||||
|             this.entryPoint, this.tsProgram.getTypeChecker(), compilation.exportReferenceGraph)); | ||||
|       } | ||||
|     } | ||||
| 
 | ||||
|     if (file === undefined) { | ||||
|       return this.diagnostics; | ||||
|     } else { | ||||
|       return this.diagnostics.filter(diag => { | ||||
|         if (diag.file === file) { | ||||
|           return true; | ||||
|         } else if (isTemplateDiagnostic(diag) && diag.componentFile === file) { | ||||
|           // Template diagnostics are reported when diagnostics for the component file are
 | ||||
|           // requested (since no consumer of `getDiagnostics` would ever ask for diagnostics from
 | ||||
|           // the fake ts.SourceFile for templates).
 | ||||
|           return true; | ||||
|         } else { | ||||
|           return false; | ||||
|         } | ||||
|       }); | ||||
|     } | ||||
|   getDiagnosticsForFile(file: ts.SourceFile, optimizeFor: OptimizeFor): ts.Diagnostic[] { | ||||
|     return [ | ||||
|       ...this.getNonTemplateDiagnostics().filter(diag => diag.file === file), | ||||
|       ...this.getTemplateDiagnosticsForFile(file, optimizeFor) | ||||
|     ]; | ||||
|   } | ||||
| 
 | ||||
|   /** | ||||
| @ -582,6 +567,37 @@ export class NgCompiler { | ||||
|     return diagnostics; | ||||
|   } | ||||
| 
 | ||||
|   private getTemplateDiagnosticsForFile(sf: ts.SourceFile, optimizeFor: OptimizeFor): | ||||
|       ReadonlyArray<ts.Diagnostic> { | ||||
|     const compilation = this.ensureAnalyzed(); | ||||
| 
 | ||||
|     // Get the diagnostics.
 | ||||
|     const typeCheckSpan = this.perfRecorder.start('typeCheckDiagnostics'); | ||||
|     const diagnostics: ts.Diagnostic[] = []; | ||||
|     if (!sf.isDeclarationFile && !this.adapter.isShim(sf)) { | ||||
|       diagnostics.push(...compilation.templateTypeChecker.getDiagnosticsForFile(sf, optimizeFor)); | ||||
|     } | ||||
| 
 | ||||
|     const program = this.typeCheckingProgramStrategy.getProgram(); | ||||
|     this.perfRecorder.stop(typeCheckSpan); | ||||
|     this.incrementalStrategy.setIncrementalDriver(this.incrementalDriver, program); | ||||
|     this.nextProgram = program; | ||||
| 
 | ||||
|     return diagnostics; | ||||
|   } | ||||
| 
 | ||||
|   private getNonTemplateDiagnostics(): ts.Diagnostic[] { | ||||
|     if (this.nonTemplateDiagnostics === null) { | ||||
|       const compilation = this.ensureAnalyzed(); | ||||
|       this.nonTemplateDiagnostics = [...compilation.traitCompiler.diagnostics]; | ||||
|       if (this.entryPoint !== null && compilation.exportReferenceGraph !== null) { | ||||
|         this.nonTemplateDiagnostics.push(...checkForPrivateExports( | ||||
|             this.entryPoint, this.tsProgram.getTypeChecker(), compilation.exportReferenceGraph)); | ||||
|       } | ||||
|     } | ||||
|     return this.nonTemplateDiagnostics; | ||||
|   } | ||||
| 
 | ||||
|   /** | ||||
|    * Reifies the inter-dependencies of NgModules and the components within their compilation scopes | ||||
|    * into the `IncrementalDriver`'s dependency graph. | ||||
|  | ||||
| @ -17,6 +17,7 @@ ts_library( | ||||
|         "//packages/compiler-cli/src/ngtsc/incremental", | ||||
|         "//packages/compiler-cli/src/ngtsc/reflection", | ||||
|         "//packages/compiler-cli/src/ngtsc/typecheck", | ||||
|         "//packages/compiler-cli/src/ngtsc/typecheck/api", | ||||
|         "@npm//typescript", | ||||
|     ], | ||||
| ) | ||||
|  | ||||
| @ -13,6 +13,7 @@ import {runInEachFileSystem} from '../../file_system/testing'; | ||||
| import {NoopIncrementalBuildStrategy} from '../../incremental'; | ||||
| import {ClassDeclaration, isNamedClassDeclaration} from '../../reflection'; | ||||
| import {ReusedProgramStrategy} from '../../typecheck'; | ||||
| import {OptimizeFor} from '../../typecheck/api'; | ||||
| 
 | ||||
| import {NgCompilerOptions} from '../api'; | ||||
| 
 | ||||
| @ -54,7 +55,8 @@ runInEachFileSystem(() => { | ||||
|           new NoopIncrementalBuildStrategy(), /** enableTemplateTypeChecker */ false, | ||||
|           /* usePoisonedData */ false); | ||||
| 
 | ||||
|       const diags = compiler.getDiagnostics(getSourceFileOrError(program, COMPONENT)); | ||||
|       const diags = compiler.getDiagnosticsForFile( | ||||
|           getSourceFileOrError(program, COMPONENT), OptimizeFor.SingleFile); | ||||
|       expect(diags.length).toBe(1); | ||||
|       expect(diags[0].messageText).toContain('does_not_exist'); | ||||
|     }); | ||||
|  | ||||
| @ -20,6 +20,7 @@ import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf'; | ||||
| import {DeclarationNode} from './reflection'; | ||||
| import {retagAllTsFiles, untagAllTsFiles} from './shims'; | ||||
| import {ReusedProgramStrategy} from './typecheck'; | ||||
| import {OptimizeFor} from './typecheck/api'; | ||||
| 
 | ||||
| 
 | ||||
| 
 | ||||
| @ -182,7 +183,9 @@ export class NgtscProgram implements api.Program { | ||||
|       } | ||||
|     } | ||||
| 
 | ||||
|     const diagnostics = this.compiler.getDiagnostics(sf); | ||||
|     const diagnostics = sf === undefined ? | ||||
|         this.compiler.getDiagnostics() : | ||||
|         this.compiler.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); | ||||
|     this.reuseTsProgram = this.compiler.getNextProgram(); | ||||
|     return diagnostics; | ||||
|   } | ||||
|  | ||||
| @ -14,6 +14,7 @@ import {NodeJSFileSystem, setFileSystem} from './file_system'; | ||||
| import {PatchedProgramIncrementalBuildStrategy} from './incremental'; | ||||
| import {NOOP_PERF_RECORDER} from './perf'; | ||||
| import {untagAllTsFiles} from './shims'; | ||||
| import {OptimizeFor} from './typecheck/api'; | ||||
| import {ReusedProgramStrategy} from './typecheck/src/augmented_program'; | ||||
| 
 | ||||
| // The following is needed to fix a the chicken-and-egg issue where the sync (into g3) script will
 | ||||
| @ -111,7 +112,10 @@ export class NgTscPlugin implements TscPlugin { | ||||
|   } | ||||
| 
 | ||||
|   getDiagnostics(file?: ts.SourceFile): ts.Diagnostic[] { | ||||
|     return this.compiler.getDiagnostics(file); | ||||
|     if (file === undefined) { | ||||
|       return this.compiler.getDiagnostics(); | ||||
|     } | ||||
|     return this.compiler.getDiagnosticsForFile(file, OptimizeFor.WholeProgram); | ||||
|   } | ||||
| 
 | ||||
|   getOptionDiagnostics(): ts.Diagnostic[] { | ||||
|  | ||||
| @ -51,7 +51,7 @@ export class LanguageService { | ||||
|       const program = compiler.getNextProgram(); | ||||
|       const sourceFile = program.getSourceFile(fileName); | ||||
|       if (sourceFile) { | ||||
|         diagnostics.push(...ttc.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile)); | ||||
|         diagnostics.push(...compiler.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile)); | ||||
|       } | ||||
|     } else { | ||||
|       const components = compiler.getComponentsWithTemplateFile(fileName); | ||||
|  | ||||
| @ -8,6 +8,7 @@ | ||||
| 
 | ||||
| import {absoluteFrom, getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system'; | ||||
| import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; | ||||
| import {OptimizeFor} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; | ||||
| 
 | ||||
| import {LanguageServiceTestEnvironment} from './env'; | ||||
| 
 | ||||
| @ -68,9 +69,15 @@ describe('language-service/compiler integration', () => { | ||||
|     // Expect that this program is clean diagnostically.
 | ||||
|     const ngCompiler = env.ngLS.compilerFactory.getOrCreate(); | ||||
|     const program = ngCompiler.getNextProgram(); | ||||
|     expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, appCmpFile))).toEqual([]); | ||||
|     expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, appModuleFile))).toEqual([]); | ||||
|     expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, testFile))).toEqual([]); | ||||
|     expect(ngCompiler.getDiagnosticsForFile( | ||||
|                getSourceFileOrError(program, appCmpFile), OptimizeFor.WholeProgram)) | ||||
|         .toEqual([]); | ||||
|     expect(ngCompiler.getDiagnosticsForFile( | ||||
|                getSourceFileOrError(program, appModuleFile), OptimizeFor.WholeProgram)) | ||||
|         .toEqual([]); | ||||
|     expect(ngCompiler.getDiagnosticsForFile( | ||||
|                getSourceFileOrError(program, testFile), OptimizeFor.WholeProgram)) | ||||
|         .toEqual([]); | ||||
|   }); | ||||
| 
 | ||||
|   it('should show type-checking errors from components with poisoned scopes', () => { | ||||
| @ -173,7 +180,8 @@ describe('language-service/compiler integration', () => { | ||||
|     // Angular should be complaining about the module not being understandable.
 | ||||
|     const programBefore = env.tsLS.getProgram()!; | ||||
|     const moduleSfBefore = programBefore.getSourceFile(moduleFile)!; | ||||
|     const ngDiagsBefore = env.ngLS.compilerFactory.getOrCreate().getDiagnostics(moduleSfBefore); | ||||
|     const ngDiagsBefore = env.ngLS.compilerFactory.getOrCreate().getDiagnosticsForFile( | ||||
|         moduleSfBefore, OptimizeFor.SingleFile); | ||||
|     expect(ngDiagsBefore.length).toBe(1); | ||||
| 
 | ||||
|     // Fix the import.
 | ||||
| @ -182,7 +190,8 @@ describe('language-service/compiler integration', () => { | ||||
|     // Angular should stop complaining about the NgModule.
 | ||||
|     const programAfter = env.tsLS.getProgram()!; | ||||
|     const moduleSfAfter = programAfter.getSourceFile(moduleFile)!; | ||||
|     const ngDiagsAfter = env.ngLS.compilerFactory.getOrCreate().getDiagnostics(moduleSfAfter); | ||||
|     const ngDiagsAfter = env.ngLS.compilerFactory.getOrCreate().getDiagnosticsForFile( | ||||
|         moduleSfAfter, OptimizeFor.SingleFile); | ||||
|     expect(ngDiagsAfter.length).toBe(0); | ||||
|   }); | ||||
| }); | ||||
|  | ||||
| @ -174,4 +174,22 @@ describe('getSemanticDiagnostics', () => { | ||||
|       'Parser Error: Bindings cannot contain assignments at column 8 in [{{nope = true}}] in /app2.html@0:0' | ||||
|     ]); | ||||
|   }); | ||||
| 
 | ||||
|   it('reports a diagnostic for a component without a template', () => { | ||||
|     const appFile = { | ||||
|       name: absoluteFrom('/app.ts'), | ||||
|       contents: ` | ||||
|       import {Component} from '@angular/core'; | ||||
|       @Component({}) | ||||
|       export class MyComponent {} | ||||
|     ` | ||||
|     }; | ||||
| 
 | ||||
|     const env = createModuleWithDeclarations([appFile]); | ||||
|     const diags = env.ngLS.getSemanticDiagnostics(absoluteFrom('/app.ts')); | ||||
| 
 | ||||
|     expect(diags.map(x => x.messageText)).toEqual([ | ||||
|       'component is missing a template', | ||||
|     ]); | ||||
|   }); | ||||
| }); | ||||
|  | ||||
| @ -11,7 +11,7 @@ import {StrictTemplateOptions} from '@angular/compiler-cli/src/ngtsc/core/api'; | ||||
| import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system'; | ||||
| import {MockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; | ||||
| import {loadStandardTestFiles} from '@angular/compiler-cli/src/ngtsc/testing'; | ||||
| import {TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; | ||||
| import {OptimizeFor, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; | ||||
| import * as ts from 'typescript/lib/tsserverlibrary'; | ||||
| 
 | ||||
| import {LanguageService} from '../language_service'; | ||||
| @ -172,7 +172,9 @@ export class LanguageServiceTestEnvironment { | ||||
|         continue; | ||||
|       } | ||||
| 
 | ||||
|       const ngDiagnostics = ngCompiler.getDiagnostics(sf); | ||||
|       // It's more efficient to optimize for WholeProgram since we call this with every file in the
 | ||||
|       // program.
 | ||||
|       const ngDiagnostics = ngCompiler.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); | ||||
|       expect(ngDiagnostics.map(diag => diag.messageText)).toEqual([]); | ||||
|     } | ||||
| 
 | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user