Skip to content

Add rewrite rule: parenthesis balance #29

@vlsi

Description

@vlsi

The parenthesis-heavy code is often hard to reason about.

For instance:

literals.add((RexLiteral) rexBuilder.makeLiteral(
    BigDecimal.ZERO, aggregateCall.getType(), false));

Even though this code might look OK, there's a readability issue.
It is not clear if BigDecimal.ZERO is an argument to .add method or .makeLiteral method.

The very same code can be reformatted as

literals.add(
    (RexLiteral) rexBuilder.makeLiteral(
        BigDecimal.ZERO, aggregateCall.getType(), false));

or as

literals.add(
    (RexLiteral) rexBuilder.makeLiteral(
        BigDecimal.ZERO,
        aggregateCall.getType(),
        false));

Both variations make it clear that .add takes a single argument.
The observation is that the code should open at most one parenthesis on a single line.

Here's a regexp-driven implementation, however, what it does it finds a line which opens more than one unclosed parenthesis (e.g. eq(gt(x,), but it can't re-format the full expression with new indentation.

Note: eq(gt(x,2),3) is not an issue because all the open parenthesis are closed on the same line


Other samples of undesired code (see autostyle/autostyle#6 ):

    if (!(group.isRows || (group.upperBound.isUnbounded()
        && group.lowerBound.isUnbounded()))) {
    builder.addRuleCollection(ImmutableList.of((RelOptRule) SubQueryRemoveRule.FILTER,
        SubQueryRemoveRule.PROJECT,
        SubQueryRemoveRule.JOIN));
      if (((nlsString.getCharset() != null
          && type.getCharset().equals(nlsString.getCharset()))
          || (nlsString.getCharset() == null
          && SqlCollation.IMPLICIT.getCharset().equals(type.getCharset())))
          && nlsString.getCollation().equals(type.getCollation())
          && ((NlsString) value).getValue().length() == type.getPrecision()) {
        includeType = RexDigestIncludeType.NO_TYPE;
      } else {
        includeType = RexDigestIncludeType.ALWAYS;
      }
    if ((leftRowCount != null)
        && (rightRowCount != null)
        && ((leftRowCount < rightRowCount)
        || ((Math.abs(leftRowCount - rightRowCount)
        < RelOptUtil.EPSILON)
        && (rowWidthCost(left.getJoinTree())
        < rowWidthCost(right.getJoinTree()))))) {
      swap = true;
    }
    Object expr4 =
        gt(
            qw(42, 43), eq(gt(x,
                y), 2));

    Object expr = gt(eq(x, 2+(3)
      ), 5);

    Object expr2 = "(" + gt(eq(x, (3)+2
      ), 6);

    Object expr3 =
        gt(
            qw(), eq(gt(x,
                y), 2));

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions