Refresh semantic diagnostics when compiler options affecting semantic diagnostics generation changes#26200
Conversation
… diagnostics generation changes Fixes #26195
| } | ||
|
|
||
| return changedCompileOptionValueOf(newOptions, oldOptions, [ | ||
| "noImplicitReturns", |
There was a problem hiding this comment.
I wonder if, instead of placing the list here, we use our central list of compiler options and mark them all with an internal field called "semantic" or something. I feel like otherwise, we will forget to update this list.
There was a problem hiding this comment.
That sounds good, but we still have these strict flags which should check their total value (from individual flag and strict flag) to see if it has really changed so we would need that part either ways.
…ting semanticDiagnostics
weswigham
left a comment
There was a problem hiding this comment.
Looks ok. esModuleInterop and allowSyntheticDefaultImports also need to be flagged, though, since the error they can report is also a semantic one.
sandersn
left a comment
There was a problem hiding this comment.
One question and one suggestion.
| return false; | ||
| } | ||
|
|
||
| for (const option of optionDeclarations) { |
There was a problem hiding this comment.
return forEach(optionDeclarations, option => return (option.stringFlag && ... reads a little better to me.
| { | ||
| name: "noUnusedParameters", | ||
| type: "boolean", | ||
| affectsSemanticDiagnostics: true, |
There was a problem hiding this comment.
What determines whether affectsSemanticDiagnostics is true?
There was a problem hiding this comment.
The flags is true if changing value of the option, affects the semantic diagnostics reported(more/less) even if there is no change in file content. ( in turn whether to we need to throw away semantic diagnostics from cache(in --w mode) ) Eg. --noUnusedLocals can report more errors when turned on so it will have the flag true
There was a problem hiding this comment.
It seems easy to forget to set this when adding a new flag. Would making it required help? It would be annoying then, though.
There was a problem hiding this comment.
yeah I don't think we want this required.
Fixes #26195