fix(compiler): properly compile DI factories when coverage reporting is enabled by JoostK · Pull Request #44732 · angular/angular

JoostK · 2022-01-16T20:17:50Z

When running tests with code coverage using Istanbul, the code is
instrumented with coverage reporting statements. These statements are
also inserted into synthesized constructors, preventing Angular from
properly recognizing them as synthesized constructor.

This commit changes the regex to detect synthesized constructors to allow
for statements within the constructor before the super(...arguments);
call. This is limited to code that does not contain a }, but this
is sufficient to support Istanbul's coverage instrumentation statements.

The tests have been extended with an input file that is being
instrumented using babel-plugin-istanbul for both ES2015 and ES5
targets, in order to verify that the approach works for real-world
usages.

Fixes #31337

…is enabled When running tests with code coverage using Istanbul, the code is instrumented with coverage reporting statements. These statements are also inserted into synthesized constructors, preventing Angular from properly recognizing them as synthesized constructor. This commit changes the regex to detect synthesized constructors to allow for statements within the constructor before the `super(...arguments);` call. This is limited to code that does not contain a `}`, but this is sufficient to support Istanbul's coverage instrumentation statements. The tests have been extended with an input file that is being instrumented using `babel-plugin-istanbul` for both ES2015 and ES5 targets, in order to verify that the approach works for real-world usages. Fixes angular#31337

…orting is enabled

JoostK · 2022-01-18T19:08:28Z

@devversion I've added you as reviewer as I'm interested in your thoughts here. I started with a full integration setup where we actually use Istanbul's Babel's transform, mainly because that was a bit more relevant to have in the original #37811. But this PR opts for a much simpler approach that avoids the problem so I figured a simple unit test is sufficient (so the original integration test setup was reverted in the fixup commit in this PR)

JoostK · 2022-01-18T22:58:01Z

Oh and adding @gkalpak so he can suggest ten ways to improve the regex 😁

devversion

@JoostK that looks great to me!

I also would have been happy with the actual istanbul transformation, but yeah, that looks like a lot of more code to maintain. I'd assume that the naming of these injected expressions etc. does not really matter in the future, neither would it seem likely that at some point the injected code before e.g. super will be dropped by istanbul.

alan-agius4

LGTM

AndrewKushnir

👍

JoostK · 2022-01-21T19:38:11Z

Presubmit isn't strictly necessary since this touches JIT-only code.

AndrewKushnir · 2022-01-21T19:41:27Z

Starting a new presubmit just to be safe :)

AndrewKushnir · 2022-01-21T21:10:46Z

This PR was merged into the repository by commit 627e807.

…is enabled (#44732) When running tests with code coverage using Istanbul, the code is instrumented with coverage reporting statements. These statements are also inserted into synthesized constructors, preventing Angular from properly recognizing them as synthesized constructor. This commit changes the regex to detect synthesized constructors to allow for statements within the constructor before the `super(...arguments);` call. This is limited to code that does not contain a `}`, but this is sufficient to support Istanbul's coverage instrumentation statements. The tests have been extended with an input file that is being instrumented using `babel-plugin-istanbul` for both ES2015 and ES5 targets, in order to verify that the approach works for real-world usages. Fixes #31337 PR Close #44732

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.1.2/13.2.1) | | [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.1.2/13.2.1) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.1.2/13.2.1) | | [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.1.2/13.2.1) | | [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.1.2/13.2.1) | | [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.1.2/13.2.1) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.1.2/13.2.1) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.1.2/13.2.1) | | [@angular/router](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.1.2/13.2.1) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v13.2.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1321-2022-02-02) [Compare Source](angular/angular@13.2.0...13.2.1) ##### animations | Commit | Type | Description | | -- | -- | -- | | [4644886aaf](angular/angular@4644886) | perf | remove no longer needed CssKeyframes classes ([#44903](angular/angular#44903)) ([#44919](angular/angular#44919)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [b4e4617807](angular/angular@b4e4617) | fix | include query parameters for open HTTP requests in `verify` ([#44917](angular/angular#44917)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [0778e6f7d7](angular/angular@0778e6f) | fix | accept nullish coalescing operator for any and unknown types ([#44862](angular/angular#44862)) | | [07185f4ed1](angular/angular@07185f4) | fix | enable nullish coalescing check only with `strictNullChecks` ([#44862](angular/angular#44862)) | | [4a5ad1793f](angular/angular@4a5ad17) | fix | ensure casing of logical paths is preserved ([#44798](angular/angular#44798)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [7ec482d9c2](angular/angular@7ec482d) | fix | Add back support for namespace URIs in createElement of dom renderer ([#44914](angular/angular#44914)) | | [250dc40a46](angular/angular@250dc40) | fix | flush delayed scoping queue while setting up TestBed ([#44814](angular/angular#44814)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [1aebbf8714](angular/angular@1aebbf8) | fix | ensure OnPush ancestors are marked dirty when the promise resolves ([#44886](angular/angular#44886)) | | [6b7fffcbeb](angular/angular@6b7fffc) | fix | Update the typed forms migration schematic to find all files. ([#44881](angular/angular#44881)) | #### Special Thanks Alan, Andrew Kushnir, Andrew Scott, Aristeidis Bampakos, Arjen, Daniel Díaz, David Shevitz, Doug Parker, Dylan Hunn, Esteban Gehring, George Kalpakas, Jessica Janiuk, JoostK, Juri Strumpflohner, Lee Robinson, Maarten Tibau, Paul Gschwendtner, Theodore Brown, arturovt, dario-piotrowicz, fru2, markostanimirovic and mgechev  ### [`v13.2.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1320-2022-01-26) [Compare Source](angular/angular@13.1.3...13.2.0) #### Deprecations ##### - The `CachedResourceLoader` and `RESOURCE_CACHE_PROVIDER` symbols were previously necessary in some cases to test AOT-compiled components with View Engine, but they are no longer needed since Ivy. - The `ComponentFactory` and `ComponentFactoryResolver` classes are deprecated. Since Ivy, there is no need to resolve Component factories. Please use other APIs where you Component classes can be used directly (without resolving their factories). - Since Ivy, the `CompilerOptions.useJit` and `CompilerOptions.missingTranslation` config options are unused, passing them has no effect. ##### | Commit | Type | Description | | -- | -- | -- | | [9c11183e74](angular/angular@9c11183) | docs | deprecate `CachedResourceLoader` and `RESOURCE_CACHE_PROVIDER` symbols ([#44749](angular/angular#44749)) | | [9f12e7fea4](angular/angular@9f12e7f) | docs | deprecate `ComponentFactory` and `ComponentFactoryResolver` symbols ([#44749](angular/angular#44749)) | | [4e95a316ce](angular/angular@4e95a31) | docs | deprecate unused config options from the `CompilerOptions` interface ([#44749](angular/angular#44749)) | ##### compiler | Commit | Type | Description | | -- | -- | -- | | [a4ab6d6b72](angular/angular@a4ab6d6) | feat | add support for safe calls in templates ([#44580](angular/angular#44580)) | | [abd1bc8039](angular/angular@abd1bc8) | fix | correct spans when parsing bindings with comments ([#44785](angular/angular#44785)) | | [ed67a074ce](angular/angular@ed67a07) | fix | properly compile DI factories when coverage reporting is enabled ([#44732](angular/angular#44732)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [fa835b5a29](angular/angular@fa835b5) | feat | enable extended diagnostics by default ([#44712](angular/angular#44712)) | | [73424def13](angular/angular@73424de) | feat | provide the animations for `DirectiveMeta` ([#44630](angular/angular#44630)) | | [fe3e4d6865](angular/angular@fe3e4d6) | fix | Handle `ng-template` with structural directive in indexer ([#44788](angular/angular#44788)) | | [7316e72ec5](angular/angular@7316e72) | fix | properly index <svg> elements when on a template ([#44785](angular/angular#44785)) | | [100091ebf0](angular/angular@100091e) | fix | remove leftover `_extendedTemplateDiagnostics` requirements ([#44777](angular/angular#44777)) | | [d2ae96f742](angular/angular@d2ae96f) | fix | skip `ExtendedTemplateCheckerImpl` construction if there were configuration errors ([#44778](angular/angular#44778)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [5626b34264](angular/angular@5626b34) | fix | consistently use namespace short name rather than URI ([#44766](angular/angular#44766)) | | [94bfcdd9de](angular/angular@94bfcdd) | fix | error if NgZone.isInAngularZone is called with a noop zone ([#44800](angular/angular#44800)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [72092ebd26](angular/angular@72092eb) | feat | Allow a FormControl to use initial value as default. ([#44434](angular/angular#44434)) | | [f7aa937cac](angular/angular@f7aa937) | fix | Make some minor fixups for forward-compatibility with typed forms. ([#44540](angular/angular#44540)) | ##### router | Commit | Type | Description | | -- | -- | -- | | [5a4ddfd4f5](angular/angular@5a4ddfd) | feat | Allow symbol keys for `Route` `data` and `resolve` properties ([#44519](angular/angular#44519)) | #### Special Thanks Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Dario Piotrowicz, Derek Cormier, Doug Parker, Douglas Parker, Dylan Hunn, George Kalpakas, Jessica Janiuk, JoostK, Kristiyan Kostadinov, Martin Probst, Oleg Postoev, Stephanie Tuerk, Tim Bowersox, Wiley Marques, Yousaf Nawaz, dario-piotrowicz, iRealNirmal, ivanwonder and shejialuo  ### [`v13.1.3`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1313-2022-01-19) [Compare Source](angular/angular@13.1.2...13.1.3) ##### animations | Commit | Type | Description | | -- | -- | -- | | [af0a152a2c](angular/angular@af0a152) | fix | apply setStyles to only rootTimelines ([#44515](angular/angular#44515)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [626f3f230b](angular/angular@626f3f2) | perf | reduce analysis work during incremental rebuilds ([#44731](angular/angular#44731)) | ##### ngcc | Commit | Type | Description | | -- | -- | -- | | [f9ca4d8499](angular/angular@f9ca4d8) | fix | support element accesses for export declarations ([#44669](angular/angular#44669)) | #### Special Thanks Alan Agius, Andrew Kushnir, AnkitSharma-007, Daniel Díaz, Dmytro Mezhenskyi, Jessica Janiuk, Joey Perrott, JoostK, Ramesh Thiruchelvam, dario-piotrowicz, iRealNirmal and Łukasz Holeczek  </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Co-authored-by: crapStone <crapstone@noreply.codeberg.org> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1148 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2022-02-21T16:06:32Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

JoostK added target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler compiler: jit labels Jan 16, 2022

ngbot bot modified the milestone: Backlog Jan 16, 2022

JoostK mentioned this pull request Jan 16, 2022

fix(compiler): use base class parameters for any non-decorated class … #44092

Closed

JoostK force-pushed the coverage-instrumentation-ctor branch 2 times, most recently from 1b14cbd to efe28b6 Compare January 16, 2022 21:05

JoostK added 2 commits January 18, 2022 19:37

fixup! fix(compiler): properly compile DI factories when coverage rep…

8e31258

…orting is enabled

JoostK force-pushed the coverage-instrumentation-ctor branch from efe28b6 to 8e31258 Compare January 18, 2022 18:38

JoostK marked this pull request as ready for review January 18, 2022 19:04

pullapprove bot requested a review from dylhunn January 18, 2022 19:05

JoostK requested a review from devversion January 18, 2022 19:05

JoostK added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jan 18, 2022

JoostK requested a review from gkalpak January 18, 2022 22:58

devversion approved these changes Jan 21, 2022

View reviewed changes

JoostK mentioned this pull request Jan 21, 2022

fix(core): handle instrumented constructors during reflection #44782

Closed

alan-agius4 approved these changes Jan 21, 2022

View reviewed changes

AndrewKushnir approved these changes Jan 21, 2022

View reviewed changes

JoostK added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 21, 2022

AndrewKushnir removed request for dylhunn and gkalpak January 21, 2022 21:08

AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Jan 21, 2022

AndrewKushnir closed this in 627e807 Jan 21, 2022

JoostK mentioned this pull request Jan 21, 2022

fix(compiler): properly compile DI factories when coverage reporting is enabled #37811

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compiler): properly compile DI factories when coverage reporting is enabled#44732

fix(compiler): properly compile DI factories when coverage reporting is enabled#44732
JoostK wants to merge 2 commits intoangular:masterfrom
JoostK:coverage-instrumentation-ctor

JoostK commented Jan 16, 2022

Uh oh!

JoostK commented Jan 18, 2022

Uh oh!

JoostK commented Jan 18, 2022

Uh oh!

devversion left a comment •

edited

Loading

Uh oh!

alan-agius4 left a comment

Uh oh!

AndrewKushnir left a comment

Uh oh!

JoostK commented Jan 21, 2022

Uh oh!

AndrewKushnir commented Jan 21, 2022

Uh oh!

AndrewKushnir commented Jan 21, 2022

Uh oh!

angular-automatic-lock-bot bot commented Feb 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

JoostK commented Jan 16, 2022

Uh oh!

JoostK commented Jan 18, 2022

Uh oh!

JoostK commented Jan 18, 2022

Uh oh!

devversion left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alan-agius4 left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

JoostK commented Jan 21, 2022

Uh oh!

AndrewKushnir commented Jan 21, 2022

Uh oh!

AndrewKushnir commented Jan 21, 2022

Uh oh!

angular-automatic-lock-bot bot commented Feb 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

devversion left a comment •

edited

Loading