Option to force uppercase SQL keywords

[othyn]

This adds a new optional third parameter to the format method, $forceUppercase which defaults to false to maintain backwards compatibility.

When passed as true, all keywords will be uppercased.

There is a new test written in for this functionality that passes.

Fixes: #32.
Implementation abstracted from the original PR against the forked source: jdorn/sql-formatter#86

[greg0ire]

How likely is it that somebody will want to sometimes uppercase SQL words, sometimes not in the same application? I wonder if this wouldn't be better as a constructor parameter. What do you think @goetas @rodnaph @othyn ?

[rodnaph]

For me, unlikely - the purpose of the tool is standardisation.

(If someone did, they could create multiple instances perhaps).

[greg0ire]

Too bad I did not use a more generic name when introducing Highlighter in #21 , this looks like it could be implemented more cleanly with a custom highlighter.

[othyn]

Man it's so long ago I can't even remember my own use case! Haha 😅 I think... it was regarding an export tool I was writing to generate formatted SQL statements from Laravel migrations, in which having uppercased keywords suited that functionality better.

As for implementation, if you think it works better as a constructor option then I'm happy to oblige.

[greg0ire]

That or maybe there should be an abstraction next to Highlighter. Highlighting is about adding things before and after, there could be another interface in charge of changing the case, so that you would have

            $highlighted = $this->highlighter->highlightToken(
                $token->type(),
                $this->changeCase($token->value())
            );

and echo (new SqlFormatter(new NullHighlighter(), new UppercaseSomethingSomething()))->format($query)

Naming ideas welcome, and let's wait for @goetas to look at this, he's good at design.

[greg0ire]

@SenseException can you please rephrase? I don't understand 😅

[SenseException]

$forceUppercase and also $indentString are used for the representation of the SQL string output and I see both as options that could be injected as a config for the second constructor argument. While $indentString has to stay in the format function for version 1, I can imagine that a future 2.0 can contain this info for "use uppercase" and "use spaces as indent" inside e.g. a Config object that SqlFormatter (or somewhere else) gets injected.

[greg0ire]

Maybe we should have a fromConfig named constructor that should call a somewhat complicated constructor in v2 yeah. PHP 8 usage could be SqlFormatter::fromConfig(indentString: '>', case_strategy: UPPERCASE_SQL_KEYWORDS, highlighter: HTML) and then it would result in the constructor being called with the right implementations, that way we don't have to maintain a config object with properties, getters and setters, it all gets validated and thrown away in the named constructor, and people can still swap in / swap out their own implementations by using the normal constructor.

[SenseException]

I like the idea of a named constructor. Not having a Config object to maintain has its advantages, but getter and setter methods aren't needed considering typed properties that are already part in 7.4.

[GingkoFr]

I sustain it, but adding a $forceUppercase option would add only ONE option.

Please read my last suggestion instead : #85