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

Indexing error in existing_or_new_singleton_class #2287

Closed
andyw8 opened this issue Jul 11, 2024 · 8 comments · Fixed by #2297
Closed

Indexing error in existing_or_new_singleton_class #2287

andyw8 opened this issue Jul 11, 2024 · 8 comments · Fixed by #2297
Assignees
Labels
bug Something isn't working server This pull request should be included in the server gem's release notes

Comments

@andyw8
Copy link
Contributor

andyw8 commented Jul 11, 2024

As reported in #1897 (comment), indexing the httplog gem is failing.

@andyw8 andyw8 added bug Something isn't working server This pull request should be included in the server gem's release notes labels Jul 11, 2024
@andyw8
Copy link
Contributor Author

andyw8 commented Jul 11, 2024

I'm still investigating but have narrowed it down to this:

# lib/httplog/adapters/http.rb

module ::HTTP
  class Client
    define_method request_method do |req, options|
      @response = send(orig_request_method, req, options)
    end
  end
end

@vinistock
Copy link
Member

I think there might be multiple layers. The top level reference on the module could be the issue. Maybe we're not deleting that prefix in some operation (since we always store entries without the leading ::).

But another possible issue might be the fact that we're indexing @response incorrectly due to the meta-programming. Since it's inside a define_method call, we're probably considering it as a class instance variable rather than an instance variable - which could also be the source of the problem.

@zachahn-gusto
Copy link

Hello!

I ran into a very similar error. I think @vinistock's comment is correct, that :: prefix isn't always being deleted. Here's an example that should break it. Removing the :: in the code fixed it.

# this errored

module ::Example1
  class Example2
    class << self
      def example3
      end
    end
  end
end

@andyw8
Copy link
Contributor Author

andyw8 commented Aug 20, 2024

@zachahn-gusto are you on the latest server gem? This does not error for me.

@zachahn-gusto
Copy link

Yes I am! 0.17.14. To be honest, I wasn't really able to reproduce this issue consistently. I'll see if I can find a better test case.

@zachahn-gusto
Copy link

Quick update, I wasn't able to make it crash, but here's a test case that demonstrates what I'm seeing

    def test_indexing_singleton
      index(<<~RUBY)
        module ::Foo
          class Bar
            class << self
            end
          end
        end
      RUBY

      # We want to explicitly verify that we didn't introduce the leading `::` by accident, but `Index#[]` deletes the
      # prefix when we use `refute_entry`
      entries = @index.instance_variable_get(:@entries)
      assert_empty(entries.keys.select { _1.start_with?("::") })
    end

That fails with this error:

Failure:
RubyIndexer::ClassesAndModulesTest#test_indexing_singleton [~/ruby-lsp/lib/ruby_indexer/test/classes_and_modules_test.rb:601]
Minitest::Assertion: Expected ["::Foo::Bar::<Class:Bar>"]
 to be empty.

@vinistock
Copy link
Member

Ah, that's another part of the bug we missed indeed. I'm cutting a PR for that.

@vinistock
Copy link
Member

#2471

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants