From 03bdee67546be70cb3ec51bc213439f842256b35 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Thu, 16 Mar 2023 03:47:29 +0000 Subject: [PATCH] [dart2js] Avoid capturing dangling else in labeled then-part Bug: https://github.com/flutter/flutter/issues/122724 Change-Id: I431bd8c1147958cefd6874187364bcf7f347c966 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289066 Reviewed-by: Nate Biggs Commit-Queue: Stephen Adams --- pkg/js_ast/lib/src/printer.dart | 8 +- pkg/js_ast/test/dangling_else_label_test.dart | 146 ++++++++++++++++++ 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 pkg/js_ast/test/dangling_else_label_test.dart diff --git a/pkg/js_ast/lib/src/printer.dart b/pkg/js_ast/lib/src/printer.dart index eda51d7d6f2..bde0cbdb4a3 100644 --- a/pkg/js_ast/lib/src/printer.dart +++ b/pkg/js_ast/lib/src/printer.dart @@ -1661,7 +1661,13 @@ class DanglingElseVisitor extends BaseVisitor { bool visitComment(Comment node) => true; @override - bool visitBlock(Block node) => false; + bool visitBlock(Block node) { + // Singleton blocks are in many places printed as the contained statement so + // that statement might capture the dangling else. + if (node.statements.length != 1) return false; + return node.statements.single.accept(this); + } + @override bool visitExpressionStatement(ExpressionStatement node) => false; @override diff --git a/pkg/js_ast/test/dangling_else_label_test.dart b/pkg/js_ast/test/dangling_else_label_test.dart new file mode 100644 index 00000000000..ac2baf74f7d --- /dev/null +++ b/pkg/js_ast/test/dangling_else_label_test.dart @@ -0,0 +1,146 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// Test for dangling-else detection when the then-part has a label. The +// then-part of an if-then-else statement sometimes needs to be wrapped in a +// block to avoid an inner if-then 'capturing' the else part. Often singleton +// blocks are printed as their contents, which can give the effect of the block +// moving upwards to the if-statement. + +import 'dart:convert'; +import 'package:expect/expect.dart'; +import 'package:js_ast/js_ast.dart'; + +void check1(String expected, Statement then) { + final actual = DebugPrint( + If(VariableUse('x'), then, ExpressionStatement(VariableUse('E')))); + Expect.equals( + expected, + actual, + '\n' + '\nexpected ${json.encode(expected)}' + '\nactual: ${json.encode(actual)}'); +} + +void check(String expected, Statement then1, + [Statement? then2, Statement? then3, Statement? then4]) { + check1(expected, then1); + if (then2 != null) check1(expected, then2); + if (then3 != null) check1(expected, then3); + if (then4 != null) check1(expected, then4); +} + +void main() { + final y = VariableUse('y'); + final z = VariableUse('z'); + final S1 = ExpressionStatement(VariableUse('S1')); + final S2 = ExpressionStatement(VariableUse('S2')); + + check( + r''' +if (x) + L: + if (y) + S1; + else + S2; +else + E; +''', + LabeledStatement('L', If(y, S1, S2)), + Block([LabeledStatement('L', If(y, S1, S2))]), + LabeledStatement('L', Block([If(y, S1, S2)])), + ); + + check( + r''' +if (x) { + L: + if (y) + S1; +} else + E; +''', + LabeledStatement('L', If.noElse(y, S1)), + Block([LabeledStatement('L', If.noElse(y, S1))]), + LabeledStatement('L', Block([If.noElse(y, S1)])), + ); + + check( + r''' +if (x) + L: { + if (y) + S1; + S2; + } +else + E; +''', + LabeledStatement('L', Block([If.noElse(y, S1), S2])), + ); + + check( + r''' +if (x) { + L: + if (y) + S1; + else if (z) + S2; +} else + E; +''', + LabeledStatement('L', If(y, S1, If.noElse(z, S2))), + Block([LabeledStatement('L', If(y, S1, If.noElse(z, S2)))]), + LabeledStatement('L', Block([If(y, S1, If.noElse(z, S2))])), + ); + + check( + r''' +if (x) { + L: + if (y) + S1; + else + if (z) + S2; +} else + E; +''', + LabeledStatement('L', If(y, S1, Block([If.noElse(z, S2)]))), + ); + + check( + r''' +if (x) { + L: + while (y) + if (z) + S1; +} else + E; +''', + LabeledStatement('L', While(y, If.noElse(z, S1))), + Block([LabeledStatement('L', While(y, If.noElse(z, S1)))]), + LabeledStatement('L', Block([While(y, If.noElse(z, S1))])), + LabeledStatement('L', While(y, Block([If.noElse(z, S1)]))), + ); + + check( + r''' +if (x) { + L: + for (;;) + if (z) + S1; +} else + E; +''', + LabeledStatement('L', For(null, null, null, If.noElse(z, S1))), + Block([LabeledStatement('L', For(null, null, null, If.noElse(z, S1)))]), + LabeledStatement('L', Block([For(null, null, null, If.noElse(z, S1))])), + LabeledStatement('L', For(null, null, null, Block([If.noElse(z, S1)]))), + ); +}