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.

Sign up for GitHub

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

Jump to bottom

Rails 6.0.0.rc1 Missing Deprecation SourceAnnotationExtractor::Annotation #36313

Closed
natevick opened this issue May 20, 2019 · 6 comments · Fixed by #36557
Closed

Rails 6.0.0.rc1 Missing Deprecation SourceAnnotationExtractor::Annotation #36313

natevick opened this issue May 20, 2019 · 6 comments · Fixed by #36557
Assignees
@sikachu
Labels
railties
Milestone
6.0.0

Comments

@natevick
Copy link
Contributor

natevick commented May 20, 2019 •
edited

I discovered this issue while upgrading a small app that uses haml_rails which is using SourceAnnotationExtractor::Annotation in version 1.

https://github.com/indirect/haml-rails/blob/86a48a59d7c31fcaacd35fdffa17551fb24e131c/lib/haml-rails.rb#L49

Here is the PR that introduced the changes #32065

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem 'rails', '6.0.0.rc1'
end

require "active_support"
require "active_support/core_ext/object/blank"
require "rails/source_annotation_extractor"
require "minitest/autorun"

def extractor
  SourceAnnotationExtractor
end

def annotation
  SourceAnnotationExtractor::Annotation
end

class SourceAnnotationExtractorTest < ActiveSupport::TestCase
  test 'test_stuff' do
    assert_not_nil extractor
  end
  test 'more_stuff' do
    assert_not_nil annotation
  end
end

Expected behavior

I would expect both tests to pass with a Deprecation warning pointing to the updated module info.

Actual behavior

The main class SourceAnnotationExtractor has the warning, but ::Annotation outputs TypeError: Rails::SourceAnnotationExtractor is not a class/module

System configuration

Rails version:
6.0.0.rc1
Ruby version:
2.6.2p47
@natevick natevick changed the title Missing Deprecation SourceAnnotationExtractor::Annotate Missing Deprecation SourceAnnotationExtractor::Annotation May 20, 2019
@natevick natevick changed the title Missing Deprecation SourceAnnotationExtractor::Annotation Rails 6.0.0.rc1 Missing Deprecation SourceAnnotationExtractor::Annotation May 21, 2019
@prathamesh-sonpatki
Copy link
Member

@nvick This is how similar issue in rspec was fixed: https://github.com/rspec/rspec-rails/blob/f227983ba2ec8c666d073f40f5a5f517510bd058/lib/rspec-rails.rb#L13
and it seems to have been fixed in haml-rails as well https://github.com/indirect/haml-rails/blob/548fa4d7140ebc39dd4a2f2fec1eb25f4ac8dc98/lib/haml-rails.rb#L51

But the fix is present in version 2 of haml-rails. This is not an Rails issue so closing it.

@natevick
Copy link
Contributor Author

natevick commented May 23, 2019 •
edited

@prathamesh-sonpatki This was caused by moving SourceAnnotationExtractor under the Rails module. The top-level class was deprecated but the sub-class was not. My tests only use Rails and no gems. Could you explain how you determined this is not a Rails issue?

@prathamesh-sonpatki
Copy link
Member

@nvick I went by the comments in #32065 and fixes made to other repos when similar issues were reported. But I will open it so others can weigh in.

@rafaelfranca rafaelfranca added this to the 6.0.0 milestone May 23, 2019
@sikachu sikachu self-assigned this May 29, 2019
@sikachu
Copy link
Member

sikachu commented May 29, 2019

This is legit break. I still owe a fix to #32065 ... so I'm assigning myself again and will be working on this.

@y-yagi
Copy link
Member

y-yagi commented Jun 26, 2019

@sikachu Are you still looking at this? If not, shall I pick up?

@sikachu sikachu mentioned this issue Jun 26, 2019
Merged
@sikachu
Copy link
Member

sikachu commented Jun 26, 2019

Sorry about the delay, I just opened #36557 to address the issue.

sikachu added a commit to sikachu/rails that referenced this issue Jul 5, 2019
@sikachu
Fix problem with accessing constant proxy subclass
48c0abb 
This commit fixes rails#36313.

After rails#32065 moved `SourceAnnotationExtractor` into `Rails` module, it
broke the ability to access `SourceAnnotationExtractor::Annotate`
directly as user would get this error:

    TypeError: Rails::SourceAnnotationExtractor is not a class/module

This commit fixes the issue by making `DeprecatedConstantProxy` to
inherit from `Module` and then defines `method_missing` and
`const_missing` to retain the previous functionality.

Thank you Matthew Draper for the idea of how to fix the issue!

[Prem Sichanugrist & Matthew Draper]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sikachu sikachu

Labels
railties
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

Fix problem with accessing deprecated constant proxy's subclass sikachu/rails
5 participants
@sikachu @rafaelfranca @natevick @prathamesh-sonpatki @y-yagi