Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make dead code indexing use a Model to index methods #565

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

Morriar
Copy link
Collaborator

@Morriar Morriar commented Jun 19, 2024

Follow-up on #564.

This time, we switch the indexing of methods to Model:

  • Remove methods indexing from Deadcode::Indexer (8464dc2)
  • Make the Deadcode::Plugins take Model::SymbolDef when calling on_define_method (7344efe)

There is 2 slight behavior changes

  1. Before we were able to look at the nesting context when defining methods inside a no_commands block in Thor classes. So with this code:

    class MyCommand < Thor
      no_commands do
        def foo; end
      end
    end

    We could tell that foo was defined inside a no_commands do ... end block and thus not ignore it as other Thor commands.

    To avoid carrying the no_commands do ... end information and putting pressure on the GC, I dropped this feature, so foo will be marked as ignored.

    This is a regression that I deemed acceptable. We can revisit this later.

  2. The Model::Builder ignores methods defined on receivers other than self. So def FOO.foo and def foo.foo will be ignored and not marked as dead. The previous indexing was clanky anyway and this form is pretty rare. We can revisit this later.

This PR is easier to review commit by commit.

@Morriar Morriar added the chore Chore task label Jun 19, 2024
@Morriar Morriar self-assigned this Jun 19, 2024
@Morriar Morriar requested a review from a team as a code owner June 19, 2024 21:02
@Morriar Morriar requested review from amomchilov and KaanOzkan June 19, 2024 21:02
@Morriar Morriar force-pushed the at-model-deadcode-constants branch from 7894f64 to 074d408 Compare June 20, 2024 14:27
@Morriar Morriar force-pushed the at-model-deadcode-methods branch from 7344efe to 9c00fa0 Compare June 20, 2024 14:28
Base automatically changed from at-model-deadcode-constants to main June 20, 2024 17:21
@Morriar Morriar merged commit 04c3408 into main Jun 20, 2024
8 checks passed
@Morriar Morriar deleted the at-model-deadcode-methods branch June 20, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Chore task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants