jcberquist / commandbox-cfformat

A CommandBox module for formatting CFML component files.
MIT License
21 stars 10 forks source link

Add setting to stop SQL formatting #81

Closed TheRealAgentK closed 4 years ago

TheRealAgentK commented 4 years ago

In script components, we usually have tag islands with <cfquery> tags containing SQL code inside.

It seems that cfformat in some instances touches and reformats the SQL code.

One example is (I added the ignore comments afterwards):

Original code

        ...
    <cfquery name="something" datasource="something">
        DECLARE @fgfhgfgh varchar(70)
        SET @fghfghfghfgh = <cfqueryparam cfsqltype="cf_sql_varchar" value="#arguments.fghfghfghfgh#">

        SELECT count(*) AS fghfghfgh
        FROM fghfghfgh C WITH (NOLOCK)
        INNER JOIN fghfghfghfgh CS WITH (NOLOCK)
            ON C.fghfghfgh = CS.fghfghfghfg AND
            CS.fghfghfghfgh = <cfqueryparam cfsqltype="cf_sql_integer" value="#arguments.fghfghfghfgh#">
        WHERE
            C.fghfghfgh = @fghfghfghfgh
            <cfif arguments.fghfghfghfghfghfghf>
                AND fghfghfghfgh != -100
            </cfif>
    </cfquery>
    ```
    ...

Running cfformat (with Ortus Solutions' code formatting guidelines) this becomes:
    ```
    <cfquery name="something" datasource="something">
      DECLARE @fgfhgfgh varchar(70)
        SET @fghfghfghfgh = <cfqueryparam cfsqltype="cf_sql_varchar" value="#arguments.fghfghfghfgh#">
        SELECT count(*) AS fghfghfgh
        FROM fghfghfgh C WITH (NOLOCK)
        INNER JOIN fghfghfghfgh CS WITH (NOLOCK)
            ON C.fghfghfghb = CS.fghfghfghfg AND
            CS.fghfghfghfgh = <cfqueryparam cfsqltype="cf_sql_integer" value="#arguments.fghfghfghfgh#">
        WHERE
            C.fghfghfgh = @fghfghfghfgh
            <cfif arguments.fghfghfghfghfghfghf>
                AND fghfghfghfgh != -100
            </cfif>
    </cfquery>
    ```

Essentially:

- removes the empty line
- pulls the indent of the first SQL line to the front (one tab)
- formats the INNER JOIN area so that the = signs are aligned.

The immediate solution is
    ```
    <!--- cfformat-ignore-start --->
    <cfquery name="something" datasource="something">
    ...
    </cfquery>
    <!--- cfformat-ignore-end --->
    ```


but it'd be much better to have the option to generally tell cfformat to leave everything in a `<cfquery>` tag alone.
jcberquist commented 4 years ago

Thanks for the detailed issue. As you note, there are a number of different things going on here.

First off, the removal off the empty line - I don't want that to happen, and that will be fixed.

The indent issue is a bit more complicated. The way it is currently handled, the cfquery tag treats the default indent level of its contents as the indent level of the tag itself and not its indent level plus one. So, currently, you will always see the first line of SQL set at that level. At the same time, it tries to respect any further lines that are indented beyond that base indent level. So you don't see the dedent of your source on subsequent lines. If your source were indented to the level of the tag and not further, things would be rendered "correctly". In looking at the issue as you have raised it, it probably makes more sense to leave the indent level of the first line alone as well.

The alignment of the inner join conditions is probably not wanted even when alignment.consecutive.assignments is true. I will see what I can do about that.

jcberquist commented 4 years ago

v0.15.7 is out, and should address these issues.

TheRealAgentK commented 4 years ago

Oh wow, awesome - thx! Will update and give it try today! :)

TheRealAgentK commented 4 years ago

0.15.7 fixes the problems I raised above, but it adds a new one in the same SQL (run with check/verbose):

                 C.ghjghjgh = @ghghjgh
-                <cfif arguments.ghjghjghj>
-                    AND ghjghjghj != -100
-                </cfif>
+                <cfif arguments.ghjghjghj>AND ghjghjghjghj != -100</cfif>
         </cfquery>

Another query that had no issues before now has empty lines inserted:

@@ -81,2 +81,3 @@
             </cfif>
+            
                 S.fghfgh AS fghfghfgh,
@@ -92,2 +93,3 @@
             </cfif>
+            
             INNER JOIN fghfghfgh S2 WITH (NOLOCK)
@@ -105,2 +107,3 @@
             </cfif>
+            
                 <!--- @CFLintIgnore CFQUERYPARAM_REQ --->
@@ -116,2 +119,3 @@
             </cfif>
+            
                 S.fghfghfg AS fghfghfgh,
@@ -127,2 +131,3 @@
             </cfif>
+            
             WHERE
@@ -136,2 +141,3 @@
             </cfif>
+            
                 <!--- @CFLintIgnore CFQUERYPARAM_REQ --->
@@ -164,2 +170,3 @@
         </cfif>
+        
         FROM fghfghfghfgh C WITH (NOLOCK)
@@ -173,2 +180,3 @@
         </cfif>
+        
         WHERE
@@ -181,2 +189,3 @@
         </cfif>
+        
             <!--- @CFLintIgnore CFQUERYPARAM_REQ --->

There are some other issues created in 0.15.7 that weren't happening before, but they are query-indpendent, so I'll raise a new ticket.

TheRealAgentK commented 4 years ago

Wouldn't a setting to globally ignore the content of <cfquery> an easier option?

jcberquist commented 4 years ago

Could you try v0.15.8 to see if the situation is improved?

TheRealAgentK commented 4 years ago

Yes, it indeed has :)

At the moment, all our queries in tag islands behave, that's awesome!

Thx for the quick help and fixes!

jcberquist commented 4 years ago

Glad to hear it is working better, thanks.