"Inline Local Variable" does not add parens when needed for subtraction/division

[DanTup]

Originally raised at Dart-Code/Dart-Code#5584 by @lukehutch.

Given the code:

void main() {
  var test = 1 + 2;
  print(3 - test);
}

The output is 0. However if you use "Inline Local" on test you get:

void main() {
  print(3 - 1 + 2);
}

Which incorrectly outputs 4.

A failing test that can be added to test\services\refactoring\legacy\inline_local_test.dart is:

  @soloTest
  Future<void> test_OK_parenthesis_plus_intoNegate() async {
    await indexTestUnit('''
void f() {
  var test = 1 + 2;
  print(3 - test);
}
''');
    _createRefactoring('test =');
    // validate change
    await assertSuccessfulRefactoring('''
void main() {
  print(3 - (1 + 2));
}
''');
  }

There's a method to determine whether to use parens here:

sdk/pkg/analysis_server/lib/src/services/refactoring/legacy/inline_local.dart

Line 230 in 560b35a

static bool _shouldUseParenthesis(Expression init, AstNode node) {

However it's not clear to me whether the bug is in that method of the Precedences from the expressions that are being compared (it doesn't seem like there are different Precendences for addition/subtraction?).

[lukehutch]

Just want to add that as the subject hints, this needs to work when a product is inlined into the divisor of div or mod too.

[bwilkerson]

it doesn't seem like there are different Precendences for addition/subtraction?

There are not. The precedence values are based on the Language Specification, which gives + and (binary) - the same precedence.

The bug here is that the test doesn't recognize that parentheses are required if the precedence of the variables's initializer and the expression in which it is being inlined are equal. That is, line 250 should use <= rather than <.

[DanTup]

Oh, I see - if that's simple I will make the fix - thanks!

[lukehutch]

The bug here is that the test doesn't recognize that parentheses are required if the precedence of the variables's initializer and the expression in which it is being inlined are equal. That is, line 250 should use <= rather than <.

That doesn't seem like the right fix, because this will unnecessarily add parens if you're inlining an addition expression into another addition expression.

This is not actually a precedence issue: within a single precedence level, it becomes an associativity issue. Addition is associative (specifically, both left-associative and right-associative); subtraction is not associative (specifically, it is defined as left-associative, and here you need to enforce right-associativity for the inlined expression).

[bwilkerson]

Absolutely. I didn't think it through far enough before replying. @DanTup put together a CL and found the problem in a test, so that's good.

[lukehutch]

As an example of why this matters: inlining into test - 3 does not need parens.