Commit Graph

10 Commits

Author SHA1 Message Date
Doug Parker e0a340ea5b refactor(compiler-cli): remove leftover _extendedTemplateDiagnostics flag (#44920)
This flag is currently a no-op because extended diagnostics are enabled in production.

PR Close #44920
2022-02-01 18:24:10 +00:00
Douglas Parker d8bff1a1b3 fix(compiler-cli): skip ExtendedTemplateCheckerImpl construction if there were configuration errors (#44778)
Previously, if a bad extended diagnostic category was given, it would fail with the expected error as well as an unexpected assertion error:

```
$ ng build -c development
✔ Browser application bundle generation complete.

./src/main.ts - Error: Module build failed (from ./node_modules/@ngtools/webpack/src/ivy/index.js):
Error: Unexpected call to 'assertNever()' with value:
test
    at /home/douglasparker/Source/ng-new/node_modules/@ngtools/webpack/src/ivy/loader.js:77:18
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

./src/polyfills.ts - Error: Module build failed (from ./node_modules/@ngtools/webpack/src/ivy/index.js):
Error: Unexpected call to 'assertNever()' with value:
test
    at /home/douglasparker/Source/ng-new/node_modules/@ngtools/webpack/src/ivy/loader.js:77:18
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

Error: error NG4004: Angular compiler option "extendedDiagnostics.checks['invalidBananaInBox']" has an unknown diagnostic category: "test".

Allowed diagnostic categories are:
warning
error
suppress
```

The assertion comes from `ExtendedTemplateCheckerImpl`, which expects a well-formed configuration, yet the compiler would construct it even when errors were found. This commit skips constructing and running extended diagnostics if the configuration had errors, which should avoid triggering these assertion errors.

I'm unfortunately not able to actually test this change. The test passes even before the fix because the `ngc` binary and end-to-end tests [don't request diagnostics unless the configuration is considered valid](https://github.com/angular/angular/blob/ed21f5c75378e1ce717ee3d76d28c8c994209de1/packages/compiler-cli/src/perform_compile.ts#L292-L293). See [Slack](https://angular-team.slack.com/archives/C4WHZQMRA/p1642641305003800) for more details.

PR Close #44778
2022-01-21 11:20:48 -08:00
Doug Parker fa835b5a29 feat(compiler-cli): enable extended diagnostics by default (#44712)
Refs #42966.

Extended diagnostics provide additional analysis about Angular templates by emitting warnings for specific patterns known to be error prone or cause developer confusion. Currently, there are two such diagnostics which are enabled by default:

* `invalidBananaInBox` emits a warning if a user writes a two-way binding backwards like `([foo])="bar"`, when they actually wanted `[(foo)]="bar"`.
* `nullishCoalescingNotNullable` emits a warning if a binding attempts to perform nullish coalescing (`??`) on a type which does not include `null` or `undefined`, such as `{{ foo ?? 'bar' }}` where `foo` is defined as `string` instead of `string | null`.

These diagnostics are enabled as warnings by default, but this can be configured in the `tsconfig.json` like so:

```jsonc
{
  "angularCompilerOptions": {
    "extendedDiagnostics": {
      // The categories to use for specific diagnostics.
      "checks": {
        // Maps check name to its category.
        "invalidBananaInBox": "suppress"
      },

      // The category to use for any diagnostics not listed in `checks` above.
      "defaultCategory": "error"
    }
  }
}
```

Allowed categories for a diagnostic are `warning` (default), `error`, or `suppress`. `warning` emits the diagnostic but allows the compilation to succeed, `error` *will* fail the compilation, while `suppress` will ignore the diagnostic altogether.

The initial release has two diagnostics, and we are hoping to expand this longer term to add more diagnostics and provide additional insight into Angular templates to detect and surface developer mistakes *before* hours of debugging are wasted.

PR Close #44712
2022-01-19 09:58:37 -08:00
Doug Parker 9e5ac51f69 refactor(compiler-cli): add defaultCategory option (#44391)
Refs #42966.

The `defaultCategory` option is used for any extended template diagnostics which do not have any specific category specified for them. It defaults to `warning`, since that is the most common behavior expected for users. This provides an easy way for users to promote all diagnostics to errors or suppress all diagnostics.

PR Close #44391
2022-01-11 17:33:16 +00:00
Doug Parker 0cf1322e4f refactor(compiler-cli): use configured diagnostic category when emitting warnings from extended template diagnostics (#44391)
Refs #42966.

This updates `TemplateContext` to include a new `makeTemplateDiagnostic()` function which automatically uses the correct diagnostic category for that check. This makes sure that each diagnostic is emitted with the correct category. It also implicitly passes some known values like `component` and `code` to make the extended template diagnostics a little simpler. Diagnostics which are suppressed are never instantiated at all, which acts as a slight performance optimization since any emitted diagnostics would be ignored anyways.

Unfortunately, diagnostics still have access to `ctx.templateTypeChecker.makeTemplateDiagnostic()` to manually create diagnostics with a different category. Both banana in box and nullish coalescing checks include tests to make sure they respect a manually configured category. This convention should hopefully give a reasonable certainty that new diagnostics will use the correct reporting function, even if that is not strictly enforced.

PR Close #44391
2022-01-11 17:33:16 +00:00
Doug Parker 1446017e4b refactor(compiler-cli): call factories directly from extended template checker (#44391)
Refs #42966.

This moves extended template check factory invocations into the checker itself, where it can provide a more consistent API contract. Factories are called with compiler options and may return a `TemplateCheck` or `undefined` if the current options do not support that check. This allows `nullishCoalescingNotNullable` to disable itself when `strictNullChecks` is disabled without throwing errors. This gives extended template diagnostics a stronger abstraction model to define their behavior with.

PR Close #44391
2022-01-11 17:33:16 +00:00
Paul Gschwendtner 8d7f1098d8 refactor: make all imports compatible with ESM/CJS output. (#43431)
As outlined in the previous commit which enabled the `esModuleInterop`
TypeScript compiler option, we need to update all namespace imports
for `typescript` to default imports. This is needed to allow for
TypeScript to be imported at runtime from an ES module.

Similar changes are needed for modules like `semver` where the types incorrectly
suggest named exports that will not exist at runtime when imported from ESM.

This commit refactors all imports to match with the lint rule we have
configured in the previous commit. See the previous commit for more
details on why certain imports have been changed.

A special case are the imports to `@babel/core` and `@babel/types`. For
these a special interop is needed as both default imports, or named
imports break the other module format. e.g default imports would work
well for ESM, but it breaks for CJS. For CJS, the named imports would
only work, but in ESM, only the default export exist. We work around
this for now until the devmode is using ESM as well (which would be
consistent with prodmode and gives us more valuable test results). More
details on the interop can be found in the `babel_core.ts` files (two
interops are needed for both localize/or the compiler-cli).

PR Close #43431
2021-10-01 18:28:45 +00:00
Daniel Trevino 95cfd0b0e1 test(compiler-cli): add tests for NullishCoalescingNotNullableCheck (#43232)
Add tests to make sure the nullish coalescing check is generating the
correct diagnostics.

Refs #42966

PR Close #43232
2021-08-26 16:36:32 -07:00
Daniel Trevino 2d8fc4a7ce refactor(compiler-cli): use TemplateDiagnostic in extended package (#43134)
Return `TemplateDiagnostic` instead of `ts.Diagnostic` when getting the
extended template diagnostics. This makes the integration with the
language service easier. This also fixes the error code and now uses the
`ngErrorCode` for extended template diagnostics.

Refs #42966

PR Close #43134
2021-08-25 10:43:00 -07:00
Daniel Trevino 4d42a3ab10 test(compiler-cli): add extended template diagnostics end-to-end tests (#43107)
This commit adds extended template diagnostics end-to-end tests, to make
sure the diagnostics are generated correctly. Template checks are
already tested with unit tests.

Refs #42966

PR Close #43107
2021-08-19 11:55:27 -07:00