Repository navigation
ruff_ruby_spo: record each Rails enum in the form the source wrote it - #137
Conversation
The Ruby walker kept a Rails enum's variants only for the hash form
`enum :status, { open: 0 }`. The array form, a constant argument and a
call with no variant argument all produced the same empty option list,
and the legacy keyword form `enum status: { open: 0 }` produced no
declaration at all. A trailing option such as `prefix: true` after an
array was captured as if it were a variant.
`Model::enum_forms` now carries one `EnumForm` per enum column beside
the unchanged legacy `AttrDecl`. `EnumVariants` distinguishes `Hash`
(key and physical value as written), `Array` (keys; the position is the
physical value), `Unresolved` (a constant or other expression, rendered
verbatim) and `Missing`. Rails enum options (`prefix`, `suffix`,
`scopes`, `default`, `validate` and their legacy underscore forms) are
kept separately and are never read as variants. The physical values
stay in this IR; OGAR never receives them as labels.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29a4ace7ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three review findings on the typed enum forms:
- `enum :status, ["open", "closed"]` stored the keys with their quotes,
so a string array produced different keys from the equivalent symbol
array. Array elements now unwrap `"str"` the way hash keys do.
- An unresolved argument such as `STATUSES.merge(extra_statuses)` was
stored as the renderer's `<expr>` placeholder, so different expressions
were indistinguishable. The walker now receives the source it parsed
and keeps the exact text; callers without the source (the event arm)
fall back to the renderer.
- A hash, keyword or array literal that splices in values
(`{ **BASE, archived: 2 }`, `[*BASE, :archived]`) was published as a
partial list missing the spliced values. Such a literal is now
`Unresolved`, with its source text.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk
|
@codex review The three findings on 29a4ace are addressed in bd202d7. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd202d7b7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The walker split keyword arguments with one option-key list for every call form, which disagrees with Rails' `enum(name = nil, values = nil, **options)`: - With a column name and no positional values, Rails reads every keyword as a variant, so `enum :status, open: 0, default: 1` declares a variant `default`; the walker had moved it to the options. - The legacy form slices off exactly `_prefix`, `_suffix`, `_scopes`, `_default` and `_instance_methods`; the walker dropped a `default:` column and turned `_instance_methods:` into a column. - A string column name (`enum "status", ..`) produced no form at all. - Hash values were rendered by the lossy argument renderer, so `Status::OPEN.to_i` became a placeholder. The modern form now treats keywords as variants when no values argument is present and as options otherwise; the legacy form uses only the underscored option keys; a string column name is accepted; and hash values and option values keep their exact source text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk
|
@codex review The three findings on bd202d7 are addressed in ae6b752. Enum argument classification now follows Rails' Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae6b752d4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A hash key or array element that needs evaluation, such as `"open".freeze`, a constant or an interpolated string, was published as a resolved variant whose key was the renderer's `<expr>` placeholder or the constant's name. Hash and array literals now resolve only when every key is a symbol or a plain string literal and nothing is splatted in; otherwise the whole argument is `Unresolved` with its source text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk
|
@codex review The computed-key finding on ae6b752 is addressed in 2953c47. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2953c47ebf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A keyword splat among enum options (`**ENUM_OPTIONS`) was discarded, so the form looked complete. EnumForm now carries unresolved_options with each splat's source text, for the modern and legacy forms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAqKMMWgnUU9g7Ldyn6sbk
|
@codex review Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fdb7c67fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let at = anchor(stmt); | ||
| let mut decls = Vec::new(); | ||
| crate::walk::walk_class_body(stmt, &mut decls); | ||
| crate::walk::walk_class_body(stmt, None, &mut decls); |
There was a problem hiding this comment.
For callers of events::source_ore, an enum such as enum :status, STATUSES.merge(extra) still produces EnumVariants::Unresolved("<expr>"), and hash values containing method calls likewise become "<expr>". Unlike the corrected model-extraction path, this newly updated event call explicitly passes None, forcing source_text to use the lossy renderer even though source_ore has the original source. Consequently, the new EnumForm payloads in ClassOre::declarations lose expressions and can disagree with the model extracted from the same file. Thread the source through the event collection path and pass it to this walker.
Useful? React with 👍 / 👎.
Summary
The Ruby walker kept a Rails enum's variants only for the hash form,
enum :status, { open: 0, closed: 1 }. The other forms lost them:enum :status, [:open, :closed](where the position is the stored value), a constant argumentenum :status, STATUSES, and a call with no variant argument all produced the same empty option list.enum status: { open: 0 }, _prefix: trueproduced no declaration at all.prefix: trueafter an array was captured as if it were a variant.Model::enum_formsnow holds oneEnumFormper enum column, alongside the legacyAttrDecl, which is unchanged.EnumVariantsrecords the form as written:HashArrayUnresolvedMissingRails enum options are kept in
EnumForm::optionsand never read as variants:prefix,suffix,scopes,default,validate, and the legacy forms_prefix,_suffix,_scopes,_default.Rails 7.1 also allows
enum :status, open: 0, closed: 1, where the keyword arguments are the variants. There, the option keys are split off and the remaining pairs becomeHashvariants. In that form a variant literally namedprefix,suffix,scopes,defaultorvalidateis read as an option.The stored values stay in this IR. OGAR's
ogar-from-ruffreads the keys only and never uses a stored value as a label. Duplicate keys are kept as written, because refusing them is the consumer's job (OGAR maps them toEnumSource::Invalid).Model::enum_formsuses#[serde(default, skip_serializing_if = "Vec::is_empty")], so dumps without the field still deserialize.EnumFormandEnumVariantsare#[non_exhaustive], with anEnumForm::newconstructor.Test plan
cargo test -p ruff_spo_triplet -p ruff_ruby_spopasses. New tests:AttrDeclis unchanged;cargo clippy -p ruff_spo_triplet -p ruff_ruby_spo --all-targets --all-features -- -D warningsis clean.cargo check --all-targetspasses forruff_csharp_spo,ruff_python_spo,ruff_spo_addressandruff_sqlalchemy_spo.Generated by Claude Code