search

doctrine / sql-formatter

A lightweight php class for formatting sql statements. Handles automatic indentation and syntax highlighting.
MIT License
1.68k stars 21 forks source link

Formatter must never fail and never add any warning #118

Open mvorisek opened 4 months ago

mvorisek commented 4 months ago

As analysed in https://github.com/doctrine/sql-formatter/pull/116#issuecomment-2125860461.

A little related with #39 formatting.

https://github.com/doctrine/sql-formatter/blob/1.4.0/src/Highlighter.php#L38-L46 should be removed then as we are very, very far validating queries (and it is impossible to do it in context-free parser).

Help welcomed.

derrabus commented 4 months ago

Formatter must never fail and never add any warning

That's nice statement, but hardly an actionable issue. Can you please elaborate a bit more what the goal of this issue is? Otherwise I'm going to close it.

mvorisek commented 4 months ago

Sure - see the linked analysis.

In this lib we do two things. We tokenize a query. And we format query. Both must be done in "we are sure" fashion and never assume anything.

What this issue namely aims is to get rid of https://github.com/doctrine/sql-formatter/blob/1.4.0/src/SqlFormatter.php#L221. This is namely the example which is currently wrong, the formatter must check if the indentation block has both start and end present, if not, no indentation must be done instead of ending it and warning about unexpected internal state.