refactor: rename Utopia\Query\Parser to Classifier - #17
Open
abnegate wants to merge 1 commit into
Open
Conversation
Two unrelated subsystems both called themselves a parser, so any file
needing both had to alias one:
Utopia\Query\Parser raw wire bytes -> Type
Utopia\Query\AST\Parser Token[] -> AST\Statement\Select
Only the second one parses. The first reads a message's leading keyword
(or, for OP_MSG, its first command name) and looks it up in a table -- it
never builds a structure. Its own method was already called
classifySQL(), and its return type is a classification enum, so the name
was the odd part, not the behaviour.
Utopia\Query\Parser -> Utopia\Query\Classifier
Utopia\Query\Parser\{SQL,MySQL,
PostgreSQL,MongoDB} -> Utopia\Query\Classifier\{...}
Classifier::parse() -> Classifier::classify()
classifySQL() and extractKeyword() keep their names: they take SQL text
rather than wire bytes, so they are a genuinely different entry point
from classify().
Classifier\MongoDB carried no-op classifySQL() and extractKeyword()
stubs -- returning Type::Unknown and '' with a "not applicable" comment
-- even though the interface never required them. MongoDB does not speak
SQL, so per the previous commits they are gone rather than answering
meaninglessly. Two tests that asserted only those no-op returns are
replaced with one asserting the methods are absent.
Both classes now carry a docblock pointing at the other, and the README
section is renamed with a note on the distinction plus links each way.
Breaking: Classifier is a public interface an external proxy would
implement. Pre-1.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Greptile SummaryThe PR consistently renames the wire-protocol query-classification API from
Confidence Score: 5/5The PR appears safe to merge, with the public rename applied consistently across implementations, tests, autoloaded paths, and documentation. No concrete blocking or independently actionable non-blocking defect remains; repository references consistently use the new API, and the intentional breaking changes are clearly documented. Important Files Changed
Reviews (1): Last reviewed commit: "refactor: rename Utopia\Query\Parser to ..." | Re-trigger Greptile |
📊 Coverage
Full per-file breakdown in the job summary. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #16 (which is stacked on #15). Review those first, or read only the single commit on this branch.
Why
Two unrelated subsystems both called themselves a parser:
Utopia\Query\ParserUtopia\Query\AST\ParserToken[]fromTokenizerType::{Read,Write,Transaction,…}AST\Statement\SelectMAX_DEPTH = 256They share no code — I checked both directions. Any file needing both had to alias one.
Only the second one parses. The first never builds a structure: its implementation is
extractKeyword()plus keyword lookup tables, its own method is already calledclassifySQL(), and its return type is a classification enum. The name was the odd part, not the behaviour.What changed
classifySQL()andextractKeyword()keep their names — they take SQL text rather than wire bytes, so they're a genuinely different entry point fromclassify(), not a redundant one.Classifier\MongoDBalso carried no-opclassifySQL()andextractKeyword()stubs, returningType::Unknownand''under a "not applicable" comment, despite the interface never requiring them. MongoDB doesn't speak SQL, so following #16 they're gone rather than answering meaninglessly. The two tests that asserted only those no-op returns become one asserting absence.Both classes now carry a docblock pointing at the other, so the distinction is discoverable from either side, and the README section is renamed with a note plus links each way.
Notes for review
Classifieris a public interface an external proxy would implement, andparse()→classify()changes its one method. Pre-1.0 (0.3.3).Tests\Query\Parser→Tests\Query\Classifier; local$parservariables renamed to$classifier. Two commit subjects quoted in aSecurityRegressionTesthistory comment still say "parser" — those are historical references and were left alone.Test plan
composer test— 5309 tests, 12345 assertions, all passcomposer check— PHPStan level max, no errorscomposer lint— passCOM_QUERYpacket and SQL text:Old names resolve to nothing (
Utopia\Query\Parser,Utopia\Query\Parser\MySQL);Utopia\Query\AST\Parseris untouched.🤖 Generated with Claude Code