Skip to content

Automatically promote constants to namespaces#505

Merged
vinistock merged 1 commit intomainfrom
01-12-remove_temporary_constant_skips_in_resolution
Feb 27, 2026
Merged

Automatically promote constants to namespaces#505
vinistock merged 1 commit intomainfrom
01-12-remove_temporary_constant_skips_in_resolution

Conversation

@vinistock
Copy link
Member

@vinistock vinistock commented Jan 22, 2026

This PR does two main things:

  • Removes all of the todos with the skips we had in resolution for constants
  • Handles some meta-programming scenarios without crashing

Although we can't provide first class features when there's meta-programming, we absolutely cannot crash. Consider these examples (or the ones added as tests):

Foo = dynamic_module

class Bar
  include Foo
end

Baz = dynamic_class

class Qux < Baz
end

module Zip; end

Boop = dynamic_class do
  include Zip
end

This PR takes two strategies to prevent crashing completely in these scenarios:

  • The most straightforward one is just skipping things that aren't namespaces during ancestor linearization. If we find an include for something that's not a namespace, we need to skip it
  • The second part is auto-promoting constants to namespaces if they get re-opened. This allows us to handle this scenario correctly (which we actually have in Core):
# At this point, we have no idea this is a class. We create a Constant declaration for it
Const = dynamic_class

# Then it gets re-opened as a class with proper includes
class Const
  include Foo
end

By auto-promoting, we correctly handle ancestors and constant resolution for Const. Of course, there's a concession being made to the user here: we trust that they didn't make a mistake by making an incompatible re-opening (i.e.: it was a module, then gets reopened as a class).

@vinistock vinistock self-assigned this Jan 22, 2026
Copy link
Member Author

vinistock commented Jan 22, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from 236f30f to f4dc87f Compare January 22, 2026 15:53
@vinistock vinistock marked this pull request as ready for review January 22, 2026 15:58
@vinistock vinistock requested a review from a team as a code owner January 22, 2026 15:58
Copy link
Contributor

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Could you rebase on either #474 or #502 (depending on the one you want to accept) so we can see the impact on diagnostics?

@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from f4dc87f to f71cd5b Compare January 23, 2026 18:21
@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from f71cd5b to 3b13abf Compare January 29, 2026 21:37
@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from 3b13abf to 1e44ea9 Compare February 18, 2026 21:35
@vinistock vinistock requested a review from Morriar February 18, 2026 21:37
@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from 1e44ea9 to 4c602d8 Compare February 23, 2026 14:59
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

I think it may be possible to further extend this approach in follow up PRs to also promote cases like:

Foo = some_method do
  include Bar # doing mixing stuff
  def baz; end # defining methods
end

In this PR, we already promote Foo when it's reopened for those operations, so IMO it makes sense to promote the above cases too. And it'd be a foundation for a basic DSL support.

@vinistock and I talked about this idea yesterday but it's not confirmed whether it'd be feasible/ideal or not.

Other than this, I think the PR is good to merge so we can index Core again.

@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from 4c602d8 to 4d2f41e Compare February 27, 2026 18:14
@vinistock
Copy link
Member Author

I added a test to document the current behaviour. Meta-programmed namespaces don't get auto promoted at the moment. That would involve inspecting to see if there are members inside, which is a bit much for this PR, but might be an avenue we want to explore for DSL handling.

@vinistock vinistock force-pushed the 01-12-remove_temporary_constant_skips_in_resolution branch from 4d2f41e to 5666886 Compare February 27, 2026 20:38
@vinistock vinistock enabled auto-merge February 27, 2026 20:39
@vinistock vinistock merged commit 82977b9 into main Feb 27, 2026
27 checks passed
@vinistock vinistock deleted the 01-12-remove_temporary_constant_skips_in_resolution branch February 27, 2026 20:42
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.

3 participants