openrewrite / rewrite-sql

OpenRewrite recipes for SQL.
Apache License 2.0
7 stars 4 forks source link

Recipe to format SQL statements inside text blocks #2

Closed MahatmaFatalError closed 1 year ago

MahatmaFatalError commented 1 year ago

Successor of https://github.com/openrewrite/rewrite-migrate-java/pull/220

timtebeek commented 1 year ago

@MahatmaFatalError I've just pushed a small change to get your recipe to run; that ought to help you work towards fixing the recipe and tests over there. :)

MahatmaFatalError commented 1 year ago

Thanks for your support. It seems the indentation is wrong. Do you know if there is a reusable component for text block indentation, maybe related to https://github.com/MahatmaFatalError/rewrite-migrate-java/blob/cedac29a8036abb6f5e69037e3088ffd394f1d8f/src/main/java/org/openrewrite/java/migrate/lang/UseTextBlocks.java#LL309C27-L309C37 ? Otherwise I would copy the code

timtebeek commented 1 year ago

That I don't know... @kunli2 might know if it makes sense to open up some of that functionality to other recipes such as this one. Until then we can start with a copy to get this PR going.

MahatmaFatalError commented 1 year ago

Alright, tried to fix the indentation problem, but the test fails with Expected recipe to complete in 1 cycle, but took 2 cycles. I don't know what to do here

timtebeek commented 1 year ago

Not quite sure about a fix for the cycles; asking a colleague to have look since I have to rush out.

shanman190 commented 1 year ago

So generally it's often better to just call JavaVisitor#autoFormat(J, ExecutionContext, Cursor) which will automatically use the detected style of the insertion point so that you don't have to do anything around finding the proper indentation type or level. It should take care of everything for you.

The cycles part is often that there is a missing if statement that prevents the recipe from looping indefinitely. You'll just want to probably detect if the formatted content is equal to the current content, then bail out with the original text block instead of updating it. Unless there's another way to check for the no work is necessary case.

timtebeek commented 1 year ago

I've managed to fix the cycle issue as described by @shanman190 ; thanks for that! I've tried maybeAutoFormat, but I suspect that does not cover text blocks, as all my attempts failed.

MahatmaFatalError commented 1 year ago

I dont know what causes this compile problem

> Task :compileJava
Compiling with JDK Java compiler API.
/home/runner/work/rewrite-sql/rewrite-sql/src/main/java/org/openrewrite/sql/table/package-info.java:20: error: cannot find symbol
import org.openrewrite.PolyglotNamespace;
                      ^
  symbol:   class PolyglotNamespace
  location: package org.openrewrite
/home/runner/work/rewrite-sql/rewrite-sql/src/main/java/org/openrewrite/sql/package-info.java:20: error: cannot find symbol
import org.openrewrite.PolyglotNamespace;
                      ^
  symbol:   class PolyglotNamespace
  location: package org.openrewrite
/home/runner/work/rewrite-sql/rewrite-sql/src/main/java/org/openrewrite/sql/table/package-info.java:16: error: cannot find symbol
@PolyglotNamespace("OpenRewrite")
 ^
  symbol: class PolyglotNamespace
/home/runner/work/rewrite-sql/rewrite-sql/src/main/java/org/openrewrite/sql/package-info.java:16: error: cannot find symbol
@PolyglotNamespace("OpenRewrite")
 ^
  symbol: class PolyglotNamespace
4 errors
timtebeek commented 1 year ago

Your branch was two commits behind on origin main and needed some changes to adopt OpenRewrite 8.0; I've applied those changes just now.

timtebeek commented 1 year ago

Thanks a lot for this recipe @MahatmaFatalError ! I've gone ahead and applied the final change I thought it needed to be able to safely run this broadly, and to separate some of the different functionality should we wish to revisit for instance the indentation later. All in all it's a very nice addition and I look forward to seeing it used in practice!

You should be able to try it out using our snapshot versions. Let me know how that works out for you! :)