alexevanczuk / packs

A pure Rust implementation of packwerk, a gradual modularization tool for Ruby
MIT License
70 stars 7 forks source link

Recurse into Const scope if unable to fetch const name #216

Closed oleg-vinted closed 1 week ago

oleg-vinted commented 2 weeks ago

I believe this fixes https://github.com/alexevanczuk/packs/issues/215.

Bar::ADAPTER['default']::INSTANCE is parsed as constant reference of INSTANCE with scope of Bar::ADAPTER['default'].

There used to be an assumption that if you encounter something other than a constant while recursing into the scope (in fetch_const_const_name), this constant reference should be ignored. However, we should instead recursively visit the scope to check if there any other constants there.

I thought of adding a test case for this, but I'm confused where to add it exactly.

Full AST of foo.rb ```rust Class( Class { name: Const( Const { scope: None, name: "Foo", double_colon_l: None, name_l: 6...9, expression_l: 6...9, }, ), superclass: None, body: Some( Def( Def { name: "foo", args: None, body: Some( Const( Const { scope: Some( Index( Index { recv: Const( Const { scope: Some( Const( Const { scope: None, name: "Bar", double_colon_l: None, name_l: 64...67, expression_l: 64...67, }, ), ), name: "ADAPTER", double_colon_l: Some( 67...69, ), name_l: 69...76, expression_l: 64...76, }, ), indexes: [ Str( Str { value: Bytes { raw: [ 100, 101, 102, 97, 117, 108, 116, ], }, begin_l: Some( 77...78, ), end_l: Some( 85...86, ), expression_l: 77...86, }, ), ], begin_l: 76...77, end_l: 86...87, expression_l: 64...87, }, ), ), name: "INSTANCE", double_colon_l: Some( 87...89, ), name_l: 89...97, expression_l: 64...97, }, ), ), keyword_l: 12...15, name_l: 16...19, end_l: Some( 100...103, ), assignment_l: None, expression_l: 12...103, }, ), ), keyword_l: 0...5, operator_l: None, end_l: 104...107, expression_l: 0...107, }, ) ```
oleg-vinted commented 1 week ago

I've added two test cases. I've confirmed that the tests fail without the code change (first commit).

alexevanczuk commented 1 week ago

Thanks @oleg-vinted ! Merging. Will push a version update for a new release.