From c79d50f38f034286fa82c21103405d471312a9be Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 31 Oct 2019 01:09:38 +0200 Subject: [PATCH] refactor(compiler-cli): avoid superfluous parenthesis around statements (#33514) Previously, due to a bug a `Context` with `isStatement: false` could be returned in places where a `Context` with `isStatement: true` was requested. As a result, some statements would be unnecessarily wrapped in parenthesis. This commit fixes the bug in `Context#withStatementMode` to always return a `Context` with the correct `isStatement` value. Note that this does not have any impact on the generated code other than avoiding some superfluous parenthesis on certain statements. PR Close #33514 --- .../src/ngtsc/translator/src/translator.ts | 2 +- .../compliance/r3_compiler_compliance_spec.ts | 48 +++++++++---------- .../compliance/r3_view_compiler_di_spec.ts | 8 ++-- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 2 +- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 947f6bd3a6..6b9b39314a 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -18,7 +18,7 @@ export class Context { get withExpressionMode(): Context { return this.isStatement ? new Context(false) : this; } - get withStatementMode(): Context { return this.isStatement ? new Context(true) : this; } + get withStatementMode(): Context { return !this.isStatement ? new Context(true) : this; } } const BINARY_OPERATORS = new Map([ diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index 9c54be9169..c0d2e29892 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -1532,8 +1532,8 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDir = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDirs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirs = $tmp$); } }, decls: 1, @@ -1591,8 +1591,8 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRef = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRefs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$); } }, … @@ -1642,8 +1642,8 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDir = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.foo = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.foo = $tmp$.first); } }, decls: 1, @@ -1708,10 +1708,10 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRef = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDir = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRefs = $tmp$); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDirs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirs = $tmp$); } }, … @@ -1769,8 +1769,8 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDir = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDirList = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$); } }, ngContentSelectors: _c0, @@ -1829,8 +1829,8 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRef = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRefs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$); } }, … @@ -1888,8 +1888,8 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDir = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.foo = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.foo = $tmp$.first); } }, ngContentSelectors: $_c1$, @@ -1955,10 +1955,10 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRef = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDir = $tmp$.first); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.myRefs = $tmp$); - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.someDirs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirs = $tmp$); } }, … @@ -3251,7 +3251,7 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.something = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$.first); } } }); @@ -3297,7 +3297,7 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.something = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$); } } }); @@ -3341,7 +3341,7 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.something = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$.first); } } }); @@ -3387,7 +3387,7 @@ describe('compiler compliance', () => { } if (rf & 2) { var $tmp$; - $r3$.ɵɵqueryRefresh(($tmp$ = $r3$.ɵɵloadQuery())) && (ctx.something = $tmp$); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.something = $tmp$); } } }); diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts index 2524f3575a..8394d0424d 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts @@ -183,9 +183,9 @@ describe('compiler compliance: dependency injection', () => { factory: function MyService_Factory(t) { var r = null; if (t) { - (r = new t()); + r = new t(); } else { - (r = (() => new MyAlternateFactory())($r3$.ɵɵinject(SomeDep))); + r = (() => new MyAlternateFactory())($r3$.ɵɵinject(SomeDep)); } return r; }, @@ -258,9 +258,9 @@ describe('compiler compliance: dependency injection', () => { factory: function MyService_Factory(t) { var r = null; if (t) { - (r = new t()); + r = new t(); } else { - (r = new MyAlternateService($r3$.ɵɵinject(SomeDep))); + r = new MyAlternateService($r3$.ɵɵinject(SomeDep)); } return r; }, diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 4c826443c3..b835975d41 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -167,7 +167,7 @@ runInEachFileSystem(os => { expect(jsContents).toContain('factory: function Service_Factory(t) { var r = null; if (t) {'); expect(jsContents).toContain('return new (t || Service)(i0.ɵɵinject(Dep));'); expect(jsContents) - .toContain('(r = (function (dep) { return new Service(dep); })(i0.ɵɵinject(Dep)));'); + .toContain('r = (function (dep) { return new Service(dep); })(i0.ɵɵinject(Dep));'); expect(jsContents).toContain('return r; }, providedIn: \'root\' });'); expect(jsContents).not.toContain('__decorate'); const dtsContents = env.getContents('test.d.ts');