Skip to content

Fix annotation placement above inner class declarations under nested_position#336

Merged
OdenTakashi merged 1 commit intodrwl:mainfrom
yamat47:fix/disambiguate-model-class
May 11, 2026
Merged

Fix annotation placement above inner class declarations under nested_position#336
OdenTakashi merged 1 commit intodrwl:mainfrom
yamat47:fix/disambiguate-model-class

Conversation

@yamat47
Copy link
Copy Markdown
Contributor

@yamat47 yamat47 commented May 9, 2026

Summary

When nested_position is enabled, AnnotationTarget (introduced in #334) picks the deepest class declaration as the insertion point. That heuristic breaks for any model that contains an inner class declaration — custom errors, value objects,
Struct/Data classes, etc. — because the deepest declaration is the inner one, so the schema annotation gets inserted directly above it instead of above the model class itself.

Reproduction

class Sample < ApplicationRecord
  include SomeMixin

  class ProcessingError < StandardError; end   # ← deepest, wrongly chosen

  def call
    raise ProcessingError
  end
end

Running annotaterb with nested_position lands the schema block right above class ProcessingError, not above class Sample.

Fix

ProjectAnnotator already resolves each model file to its actual ActiveRecord class via ModelClassGetter, so we have the authoritative class name without needing any heuristic. This PR threads klass.name.demodulize through:

SingleFileAnnotatorInstructionSingleFileAnnotator
ParsedFile / AnnotatedFile::Generator
FileParser::AnnotationTarget.find

as model_class_name:, and AnnotationTarget.find matches that name against parser.type_map first.

The deepest-declaration heuristic remains as a fallback for the cases where the name is absent or doesn't match anything in the source:

  • related files (specs, factories, fixtures) don't get a model_class_name
  • legacy file/class layouts where the declared class name doesn't match the resolved AR class name

So existing behavior is preserved everywhere except the inner-class case.

drwl#334 introduced `AnnotationTarget` and made it pick the
deepest class declaration when `nested_position` is on. That heuristic
is wrong for models containing inner class declarations (custom errors,
value objects, Struct/Data classes, …): the deepest declaration is the
inner one, and the schema annotation gets inserted directly above it
instead of above the model class.

```ruby
class Sample < ApplicationRecord
  include SomeMixin

  class ProcessingError < StandardError; end   # ← deepest, wrongly chosen

  def call
    raise ProcessingError
  end
end
```

`ProjectAnnotator` already resolves each model file to its actual
ActiveRecord class via `ModelClassGetter`, so the authoritative class
name is available without any heuristic. Thread `klass.name.demodulize`
through `SingleFileAnnotatorInstruction` → `SingleFileAnnotator` →
`ParsedFile`/`Generator` → `AnnotationTarget.find` as `model_class_name:`,
and have the helper match that name against `parser.type_map` first.

The deepest-declaration heuristic remains as a fallback for when the
name is absent (related files like specs and factories) or doesn't
match anything in the source (legacy file/class layouts).
@yamat47 yamat47 marked this pull request as ready for review May 9, 2026 10:20
Copy link
Copy Markdown
Collaborator

@OdenTakashi OdenTakashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this. Looks good to me 🚀
Your detailed explanation made it very easy to understand.

@OdenTakashi OdenTakashi merged commit 3413466 into drwl:main May 11, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants