Skip to content

Commit

Permalink
refactor(compiler-cli): avoid superfluous parenthesis around statemen…
Browse files Browse the repository at this point in the history 
…ts (#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
  • Loading branch information
@gkalpak @kara
gkalpak authored and kara committed Nov 13, 2019
1 parent 624425a commit c79d50f
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 30 deletions.
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/translator/src/translator.ts
View file
Expand Up @@ -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<BinaryOperator, ts.BinaryOperator>([
Expand Down
48 changes: 24 additions & 24 deletions packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
View file
Expand Up @@ -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,
Expand Down Expand Up @@ -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$);
}
},
…
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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$);
}
},
…
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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$);
}
},
…
Expand Down Expand Up @@ -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$,
Expand Down Expand Up @@ -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$);
}
},
…
Expand Down Expand Up @@ -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);
}
}
});
Expand Down Expand Up @@ -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$);
}
}
});
Expand Down Expand Up @@ -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);
}
}
});
Expand Down Expand Up @@ -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$);
}
}
});
Expand Down
8 changes: 4 additions & 4 deletions packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts
View file
Expand Up @@ -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;
},
Expand Down Expand Up @@ -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;
},
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View file
Expand Up @@ -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');
Expand Down

0 comments on commit c79d50f

Please sign in to comment.