drccrd commented on code in PR #4005: URL: https://github.com/apache/incubator-kie-tools/pull/4005#discussion_r4004133237
########## packages/drools-lsp/drools-formatter/src/main/java/org/drools/formatter/LhsFormatter.java: ########## @@ -0,0 +1,671 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.drools.formatter; + +import java.util.List; + +import org.antlr.v4.runtime.Token; +import org.drools.drl.parser.antlr4.DRL10Lexer; +import org.drools.drl.parser.antlr4.DRL10Parser; + +/** + * Formats a rule's conditions (the when block) and a query's body. + * + * <p>This class walks the CST with typed {@code visit*} methods: + * <ul> + * <li>{@code visitLhs} → {@code visitLhsExpression} dispatches on + * expression type (and/or/unary/enclosed).</li> + * <li>{@code visitLhsUnary} handles patterns, not/exists, accumulate, + * forall, eval, and groupby.</li> + * <li>{@code visitPatternBind} renders a pattern on one line if it fits + * within {@code options.lineLength()} and its constraint parens hold no line + * comment (a {@code //} comment would swallow the rest of a one-line + * rendering); otherwise delegates to {@code emitReflowedPattern} which + * puts each constraint on its own line — trailing comments attached — + * with the closing {@code )} aligned to the opening.</li> + * <li>Parentheses are padded — {@code Person( age > 18 )} — whatever the + * source did, an empty pair staying {@code ()}. The rule is enforced in + * three places because parens reach the output three ways: token-by-token + * emission ({@code needsSpaceBetween}), verbatim RHS code + * ({@code rhsNeedsSpace}), and string-built signatures + * ({@code parenthesized}).</li> + * <li>{@code visitAndDef} emits explicit {@code and} keywords between + * patterns in accumulate/groupby source via a {@code nextPatternPrefix} + * mechanism.</li> + * </ul> + */ +final class LhsFormatter { + private final Emitter e; + + // Prefix to prepend to the next pattern (e.g. "and " between patterns) + private String nextPatternPrefix = ""; + + LhsFormatter(Emitter e) { + this.e = e; + } + + void visitLhs(DRL10Parser.LhsContext ctx) { + e.emitHiddenTokensBefore(ctx); + e.emit(e.indent() + "when"); + e.newline(); + e.depth++; + for (DRL10Parser.LhsExpressionContext expr : ctx.lhsExpression()) { + visitLhsExpression(expr); + } + e.depth--; + } + + void visitLhsExpression(DRL10Parser.LhsExpressionContext ctx) { + e.emitHiddenTokensBefore(ctx); + if (ctx instanceof DRL10Parser.LhsExpressionEnclosedContext enclosed) { + e.emit(e.indent() + consumePatternPrefix() + "("); + e.newline(); + e.depth++; + visitLhsExpression(enclosed.lhsExpression()); + e.depth--; + e.emit(e.indent() + ")"); + e.newline(); + } else if (ctx instanceof DRL10Parser.LhsOrContext orCtx) { + List<DRL10Parser.LhsExpressionContext> exprs = orCtx.lhsExpression(); + for (int i = 0; i < exprs.size(); i++) { + if (i > 0) { + e.emit(e.indent() + "or"); + e.newline(); + } + visitLhsExpression(exprs.get(i)); + } + } else if (ctx instanceof DRL10Parser.LhsAndContext andCtx) { + List<DRL10Parser.LhsExpressionContext> exprs = andCtx.lhsExpression(); + for (int i = 0; i < exprs.size(); i++) { + if (i > 0) { + nextPatternPrefix = "and "; + } + visitLhsExpression(exprs.get(i)); + } + } else if (ctx instanceof DRL10Parser.LhsUnarySingleContext unary) { + visitLhsUnary(unary.lhsUnary()); + } + } + + private void visitLhsUnary(DRL10Parser.LhsUnaryContext ctx) { + e.emitHiddenTokensBefore(ctx); + // Consume any pending prefix (e.g. "and ") for non-pattern nodes. + // visitPatternBind consumes it itself; other handlers need it here. + if (ctx.lhsPatternBind() != null) { + visitPatternBind(ctx.lhsPatternBind()); + emitTrailingInvocations(ctx); + return; + } + String prefix = consumePatternPrefix(); + if (ctx.lhsExists() != null) { + visitLhsExists(ctx.lhsExists(), prefix); + } else if (ctx.lhsNot() != null) { + visitLhsNot(ctx.lhsNot(), prefix); + } else if (ctx.lhsEval() != null) { + String inner = e.styledText(ctx.lhsEval().conditionalOrExpression()); + e.emit(e.indent() + prefix + + (e.options.parenPadding() ? "eval( " + inner + " )" : "eval(" + inner + ")")); + e.newline(); + } else if (ctx.lhsForall() != null) { + visitLhsForall(ctx.lhsForall()); + } else if (ctx.lhsAccumulate() != null) { + visitLhsAccumulate(ctx.lhsAccumulate()); + } else if (ctx.lhsGroupBy() != null) { + visitLhsGroupBy(ctx.lhsGroupBy()); + } else if (ctx.lhsExpression() != null) { + // parenthesized lhsExpression + e.emit(e.indent() + prefix + "("); + e.newline(); + e.depth++; + visitLhsExpression(ctx.lhsExpression()); + e.depth--; + e.emit(e.indent() + ")"); + e.newline(); + } else if (ctx.conditionalBranch() != null) { + e.emit(e.indent() + prefix + e.styledText(ctx.conditionalBranch())); + e.newline(); + } + // exists/not/eval and a parenthesised group can each carry a trailing + // invocation too; the pattern-bind path above appends its own. + emitTrailingInvocations(ctx); + } + + /** + * The trailing consequence invocations of an {@code lhsUnary}, rendered for + * the end of the element's line. + * + * <p>The grammar hangs these off {@code lhsUnary} as siblings of the element + * they qualify — {@code lhsPatternBind consequenceInvocation*}, + * {@code lhsExists namedConsequenceInvocation?} — so visiting only the + * element drops them. Dropping one is silent: {@code $p : Bar( age > 18 ) if + * ( $p.age > 65 ) do[senior]} loses its guard while the {@code then[senior]} + * block survives, leaving a labelled consequence that can never fire, and the + * output still re-parses, so the output-reparse net cannot see it. + */ + private String trailingInvocations(DRL10Parser.LhsUnaryContext ctx) { + StringBuilder sb = new StringBuilder(); + for (DRL10Parser.ConsequenceInvocationContext invocation : ctx.consequenceInvocation()) { + sb.append(' ').append(e.styledText(invocation)); + } + if (ctx.namedConsequenceInvocation() != null) { + sb.append(' ').append(e.styledText(ctx.namedConsequenceInvocation())); + } + return sb.toString(); + } + + /** + * Attach an {@code lhsUnary}'s trailing invocations to the element just + * emitted. The element may have taken several lines (reflowed, or forced + * multi-line by a comment), so this appends to the emitted text rather than + * to a current-line buffer. + */ + private void emitTrailingInvocations(DRL10Parser.LhsUnaryContext ctx) { + String trailing = trailingInvocations(ctx); + if (trailing.isEmpty()) { + return; + } + e.appendToLastLine(trailing); + e.newline(); + } + + private void visitLhsExists(DRL10Parser.LhsExistsContext ctx, String prefix) { + if (ctx.lhsPatternBind() != null) { + if (emitMultiLineIfCommented(ctx.lhsPatternBind(), prefix + "exists ")) { + return; + } + String patternText = formatPatternBind(ctx.lhsPatternBind()); + String line = e.indent() + prefix + "exists " + patternText; + if (line.length() <= e.options.lineLength()) { + e.emit(line); + e.newline(); + } else { + emitReflowedPattern(ctx.lhsPatternBind(), prefix + "exists "); + } + } else if (ctx.lhsExpression() != null) { + e.emit(e.indent() + prefix + "exists("); + e.newline(); + e.depth++; + visitLhsExpression(ctx.lhsExpression()); + e.depth--; + e.emit(e.indent() + ")"); + e.newline(); + } + } + + private void visitLhsNot(DRL10Parser.LhsNotContext ctx, String prefix) { + if (ctx.lhsPatternBind() != null) { + if (emitMultiLineIfCommented(ctx.lhsPatternBind(), prefix + "not ")) { + return; + } + String patternText = formatPatternBind(ctx.lhsPatternBind()); + String line = e.indent() + prefix + "not " + patternText; + if (line.length() <= e.options.lineLength()) { + e.emit(line); + e.newline(); + } else { + emitReflowedPattern(ctx.lhsPatternBind(), prefix + "not "); + } + } else if (ctx.lhsExpression() != null) { + e.emit(e.indent() + prefix + "not("); + e.newline(); + e.depth++; + visitLhsExpression(ctx.lhsExpression()); + e.depth--; + e.emit(e.indent() + ")"); + e.newline(); + } + } + + private void visitLhsForall(DRL10Parser.LhsForallContext ctx) { + e.emit(e.indent() + "forall("); + e.newline(); + e.depth++; + for (DRL10Parser.LhsPatternBindContext pb : ctx.lhsPatternBind()) { + visitPatternBind(pb); + } + e.depth--; + e.emit(e.indent() + ")"); + e.newline(); + } + + private void visitLhsAccumulate(DRL10Parser.LhsAccumulateContext ctx) { + String keyword = ctx.DRL_ACC() != null ? "acc" : "accumulate"; + e.emit(e.indent() + keyword + "("); + e.newline(); + e.depth++; + // The grammar accepts "," or ";" after the source pattern, but the DRL + // language reference documents inline accumulate as + // accumulate( <source pattern>; <functions> [;<constraints>] ) + // so a source "," is normalized to the documented ";" form. + visitAndDef(ctx.lhsAndDef()); + e.appendToLastLine(e.options.normalizeTerminators() ? ";" : sourceSeparator(ctx.lhsAndDef())); + e.newline(); + // accumulate functions + List<DRL10Parser.AccumulateFunctionContext> funcs = ctx.accumulateFunction(); + for (int i = 0; i < funcs.size(); i++) { + e.emit(e.indent() + e.styledText(funcs.get(i))); Review Comment: I actively decided for this behavior during development. I don't think supporting comments makes sense here, it would make the code less readable and a comment can just be placed before the pattern. I don't think a change is needed here but maybe ensure it is documented. -> https://github.com/apache/incubator-kie-tools/pull/4005/changes#diff-7d8703ff9179fa3c7d45c4747bd4fc7526a14b595f903a1b444260f0573d0916R191 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
