Compare commits

..

17 Commits

Author SHA1 Message Date
Charlie Somerville
24e5712294 Merge pull request #26 from github/kill-whiny-nils
Kill whiny nils
2013-10-29 20:32:13 -07:00
Charlie Somerville
8f6bafc333 💀 whiny nils 2013-10-29 20:25:48 -07:00
Charlie Somerville
c717a84b5d Merge pull request #24 from github/avoid-extension-when-instantiating-extended-association
Avoid extension when instantiating extended association
2013-10-29 20:23:28 -07:00
Charlie Somerville
d537304b20 replace :: with _ to avoid wrong constant name exceptions 2013-10-29 20:16:52 -07:00
Charlie Somerville
ca90ecf2cb use terrible hacks to make this work when rails tries to marshal 2013-10-29 20:06:11 -07:00
Charlie Somerville
4bb1d3ef20 cache a class with the extend module pre-included 2013-10-29 20:06:11 -07:00
John Barnette
3b7754c950 Merge pull request #25 from github/activesupport-concern
Pull in ActiveSupport::Concern
2013-10-29 12:10:45 -07:00
John Barnette
75638c576b Pull in ActiveSupport::Concern
We have quite a few module dependency situations that this can help
clarify.
2013-10-29 12:03:54 -05:00
Charlie Somerville
76884dd7f7 Merge pull request #22 from github/actionview-proxy-module-method-cache-nuke
Don't globally invalidate the method and constant cache every view render
2013-10-25 11:43:48 -07:00
Charlie Somerville
29a72262aa here too 2013-10-25 12:46:48 -04:00
Charlie Somerville
76c5bf4f4b instantiate the cached helper class instead of extending AV::B 2013-10-25 12:46:48 -04:00
Charlie Somerville
416b7171b8 delete ActionView::Base#helpers because it's completely useless 2013-10-25 12:46:48 -04:00
Charlie Somerville
e82a3ba2a0 cache a class that is pre-included with the master helper module 2013-10-25 12:46:48 -04:00
Charlie Somerville
8837faac73 Merge pull request #21 from github/kill-blankslate
Kill blankslate
2013-10-25 09:42:01 -07:00
Charlie Somerville
20b12c3b42 call Kernel.block_given? instead of block_given? coz of BasicObject 2013-10-24 14:30:20 -04:00
Charlie Somerville
0cf06787af use fully qualified constant access here 2013-10-24 14:30:20 -04:00
Charlie Somerville
5efad05b11 💀 in a 🔥 blankslate 2013-10-24 14:30:20 -04:00
18 changed files with 284 additions and 311 deletions

View File

@@ -1273,8 +1273,7 @@ module ActionController #:nodoc:
end
def initialize_template_class(response)
response.template = ActionView::Base.new(self.class.view_paths, {}, self)
response.template.helpers.send :include, self.class.master_helper_module
response.template = self.class.master_helper_class.new(self.class.view_paths, {}, self)
response.redirected_to = nil
@performed_render = @performed_redirect = false
end

View File

@@ -69,6 +69,22 @@ module ActionController #:nodoc:
# N/A | Carolina Railhaws Training Workshop
#
module ClassMethods
# To avoid extending an instance of ActionView::Base with the master_helper_module
# every single time we render a view, we're caching a class that has
# master_helper_module already included that we can just instantiate.
def master_helper_class
return @master_helper_class if @master_helper_class
@master_helper_class = Class.new(ActionView::Base).tap do |klass|
klass.send(:include, master_helper_module)
end
end
def master_helper_module=(mod)
write_inheritable_attribute(:master_helper_module, mod)
@master_helper_class = nil
end
# Makes all the (instance) methods in the helper module available to templates rendered through this controller.
# See ActionView::Helpers (link:classes/ActionView/Helpers.html) for more about making your own helper modules
# available to the templates.
@@ -182,8 +198,7 @@ module ActionController #:nodoc:
# Provides a proxy to access helpers methods from outside the view.
def helpers
unless @helper_proxy
@helper_proxy = ActionView::Base.new
@helper_proxy.extend master_helper_module
@helper_proxy = master_helper_class.new
else
@helper_proxy
end

View File

@@ -203,24 +203,10 @@ module ActionView #:nodoc:
ActionView::PathSet.new(Array(value))
end
attr_reader :helpers
class ProxyModule < Module
def initialize(receiver)
@receiver = receiver
end
def include(*args)
super(*args)
@receiver.extend(*args)
end
end
def initialize(view_paths = [], assigns_for_first_render = {}, controller = nil)#:nodoc:
@assigns = assigns_for_first_render
@assigns_added = nil
@controller = controller
@helpers = ProxyModule.new(self)
self.view_paths = view_paths
@_first_render = nil

View File

@@ -116,8 +116,7 @@ module ActionView
end
def _view
view = ActionView::Base.new(ActionController::Base.view_paths, _assigns, @controller)
view.helpers.include master_helper_module
view = self.class.master_helper_class.new(ActionController::Base.view_paths, _assigns, @controller)
view.output_buffer = self.output_buffer
view
end

View File

@@ -47,14 +47,29 @@ module ActiveRecord
# instantiation of the actual post records.
class AssociationProxy #:nodoc:
alias_method :proxy_respond_to?, :respond_to?
alias_method :proxy_extend, :extend
delegate :to_param, :to => :proxy_target
instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id)$|^__|^respond_to_missing|proxy_/ }
def self.new(owner, reflection)
klass =
reflection.cached_extend_class ||=
if reflection.options[:extend]
const_name = "AR_CACHED_EXTEND_CLASS_#{reflection.name}_#{reflection.options[:extend].join("_").gsub("::","_")}"
reflection.active_record.const_set(const_name, Class.new(self) do
include *reflection.options[:extend]
end)
else
self
end
proxy = klass.allocate
proxy.send(:initialize, owner, reflection)
proxy
end
def initialize(owner, reflection)
@owner, @reflection = owner, reflection
reflection.check_validity!
Array(reflection.options[:extend]).each { |ext| proxy_extend(ext) }
reset
end

View File

@@ -77,6 +77,7 @@ module ActiveRecord
# those classes. Objects of AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods.
class MacroReflection
attr_reader :active_record
attr_accessor :cached_extend_class
def initialize(macro, name, options, active_record)
@macro, @name, @options, @active_record = macro, name, options, active_record

View File

@@ -0,0 +1,134 @@
module ActiveSupport
# A typical module looks like this:
#
# module M
# def self.included(base)
# base.extend ClassMethods
# base.class_eval do
# scope :disabled, -> { where(disabled: true) }
# end
# end
#
# module ClassMethods
# ...
# end
# end
#
# By using <tt>ActiveSupport::Concern</tt> the above module could instead be
# written as:
#
# require 'active_support/concern'
#
# module M
# extend ActiveSupport::Concern
#
# included do
# scope :disabled, -> { where(disabled: true) }
# end
#
# module ClassMethods
# ...
# end
# end
#
# Moreover, it gracefully handles module dependencies. Given a +Foo+ module
# and a +Bar+ module which depends on the former, we would typically write the
# following:
#
# module Foo
# def self.included(base)
# base.class_eval do
# def self.method_injected_by_foo
# ...
# end
# end
# end
# end
#
# module Bar
# def self.included(base)
# base.method_injected_by_foo
# end
# end
#
# class Host
# include Foo # We need to include this dependency for Bar
# include Bar # Bar is the module that Host really needs
# end
#
# But why should +Host+ care about +Bar+'s dependencies, namely +Foo+? We
# could try to hide these from +Host+ directly including +Foo+ in +Bar+:
#
# module Bar
# include Foo
# def self.included(base)
# base.method_injected_by_foo
# end
# end
#
# class Host
# include Bar
# end
#
# Unfortunately this won't work, since when +Foo+ is included, its <tt>base</tt>
# is the +Bar+ module, not the +Host+ class. With <tt>ActiveSupport::Concern</tt>,
# module dependencies are properly resolved:
#
# require 'active_support/concern'
#
# module Foo
# extend ActiveSupport::Concern
# included do
# def self.method_injected_by_foo
# ...
# end
# end
# end
#
# module Bar
# extend ActiveSupport::Concern
# include Foo
#
# included do
# self.method_injected_by_foo
# end
# end
#
# class Host
# include Bar # works, Bar takes care now of its dependencies
# end
module Concern
class MultipleIncludedBlocks < StandardError #:nodoc:
def initialize
super "Cannot define multiple 'included' blocks for a Concern"
end
end
def self.extended(base) #:nodoc:
base.instance_variable_set(:@_dependencies, [])
end
def append_features(base)
if base.instance_variable_defined?(:@_dependencies)
base.instance_variable_get(:@_dependencies) << self
return false
else
return false if base < self
@_dependencies.each { |dep| base.send(:include, dep) }
super
base.extend const_get(:ClassMethods) if const_defined?(:ClassMethods)
base.class_eval(&@_included_block) if instance_variable_defined?(:@_included_block)
end
end
def included(base = nil, &block)
if base.nil?
raise MultipleIncludedBlocks if instance_variable_defined?(:@_included_block)
@_included_block = block
else
super
end
end
end
end

View File

@@ -1,114 +0,0 @@
#!/usr/bin/env ruby
#--
# Copyright 2004, 2006 by Jim Weirich (jim@weirichhouse.org).
# All rights reserved.
# Permission is granted for use, copying, modification, distribution,
# and distribution of modified versions of this work as long as the
# above copyright notice is included.
#++
######################################################################
# BlankSlate provides an abstract base class with no predefined
# methods (except for <tt>\_\_send__</tt> and <tt>\_\_id__</tt>).
# BlankSlate is useful as a base class when writing classes that
# depend upon <tt>method_missing</tt> (e.g. dynamic proxies).
#
class BlankSlate
class << self
# Hide the method named +name+ in the BlankSlate class. Don't
# hide +instance_eval+ or any method beginning with "__".
def hide(name)
if (instance_methods.include?(name) || instance_methods.include?(name.to_sym)) and
name !~ /^(__|instance_eval|object_id)/
@hidden_methods ||= {}
@hidden_methods[name] = instance_method(name)
undef_method name
end
end
def find_hidden_method(name)
@hidden_methods ||= {}
@hidden_methods[name] || superclass.find_hidden_method(name)
end
# Redefine a previously hidden method so that it may be called on a blank
# slate object.
def reveal(name)
bound_method = nil
unbound_method = find_hidden_method(name)
fail "Don't know how to reveal method '#{name}'" unless unbound_method
define_method(name) do |*args|
bound_method ||= unbound_method.bind(self)
bound_method.call(*args)
end
end
end
instance_methods.each { |m| hide(m) }
end
######################################################################
# Since Ruby is very dynamic, methods added to the ancestors of
# BlankSlate <em>after BlankSlate is defined</em> will show up in the
# list of available BlankSlate methods. We handle this by defining a
# hook in the Object and Kernel classes that will hide any method
# defined after BlankSlate has been loaded.
#
module Kernel
class << self
alias_method :blank_slate_method_added, :method_added
# Detect method additions to Kernel and remove them in the
# BlankSlate class.
def method_added(name)
result = blank_slate_method_added(name)
return result if self != Kernel
BlankSlate.hide(name)
result
end
end
end
######################################################################
# Same as above, except in Object.
#
class Object
class << self
alias_method :blank_slate_method_added, :method_added
# Detect method additions to Object and remove them in the
# BlankSlate class.
def method_added(name)
result = blank_slate_method_added(name)
return result if self != Object
BlankSlate.hide(name)
result
end
def find_hidden_method(name)
nil
end
end
end
######################################################################
# Also, modules included into Object need to be scanned and have their
# instance methods removed from blank slate. In theory, modules
# included into Kernel would have to be removed as well, but a
# "feature" of Ruby prevents late includes into modules from being
# exposed in the first place.
#
class Module
alias blankslate_original_append_features append_features
def append_features(mod)
result = blankslate_original_append_features(mod)
return result if mod != Object
instance_methods.each do |name|
BlankSlate.hide(name)
end
result
end
end

View File

@@ -1,20 +0,0 @@
#!/usr/bin/env ruby
#--
# Copyright 2004, 2006 by Jim Weirich (jim@weirichhouse.org).
# All rights reserved.
# Permission is granted for use, copying, modification, distribution,
# and distribution of modified versions of this work as long as the
# above copyright notice is included.
#++
require 'blankslate'
######################################################################
# BlankSlate has been promoted to a top level name and is now
# available as a standalone gem. We make the name available in the
# Builder namespace for compatibility.
#
module Builder
BlankSlate = ::BlankSlate
end

View File

@@ -18,8 +18,6 @@
# Style Sheets (CSS).
require 'builder/blankslate'
module Builder
# Create a Cascading Style Sheet (CSS) using Ruby.
@@ -89,7 +87,7 @@ module Builder
# background: red;
# }
#
class CSS < BlankSlate
class CSS < BasicObject
# Create a CSS builder.
#
@@ -155,7 +153,7 @@ module Builder
def group!(*args, &block)
args.each do |arg|
if arg.is_a?(Symbol)
if arg.is_a?(::Symbol)
instance_eval(&@library[arg])
else
instance_eval(&arg)
@@ -169,7 +167,7 @@ module Builder
end
def method_missing(sym, *args, &block)
sym = "#{sym}:#{args.shift}" if args.first.kind_of?(Symbol)
sym = "#{sym}:#{args.shift}" if args.first.kind_of?(::Symbol)
if block
_start_container(sym, args.first)
_css_block(block)

View File

@@ -1,7 +1,5 @@
#!/usr/bin/env ruby
require 'builder/blankslate'
module Builder
# Generic error for builder
@@ -9,7 +7,7 @@ module Builder
# XmlBase is a base class for building XML builders. See
# Builder::XmlMarkup and Builder::XmlEvents for examples.
class XmlBase < BlankSlate
class XmlBase < BasicObject
# Create an XML markup builder.
#
@@ -37,10 +35,10 @@ module Builder
def method_missing(sym, *args, &block)
text = nil
attrs = nil
sym = "#{sym}:#{args.shift}" if args.first.kind_of?(Symbol)
sym = "#{sym}:#{args.shift}" if args.first.kind_of?(::Symbol)
args.each do |arg|
case arg
when Hash
when ::Hash
attrs ||= {}
attrs.merge!(arg)
else
@@ -50,7 +48,7 @@ module Builder
end
if block
unless text.nil?
raise ArgumentError, "XmlMarkup cannot mix a text argument with a block"
raise ::ArgumentError, "XmlMarkup cannot mix a text argument with a block"
end
_indent
_start_tag(sym, attrs)

View File

@@ -195,7 +195,7 @@ module Builder
end
def comment!(comment_text)
_ensure_no_block block_given?
_ensure_no_block ::Kernel.block_given?
_special("<!-- ", " -->", comment_text, nil)
end
@@ -210,13 +210,13 @@ module Builder
@target << "<!#{inst}"
args.each do |arg|
case arg
when String
when ::String
@target << %{ "#{arg}"} # " WART
when Symbol
when ::Symbol
@target << " #{arg}"
end
end
if block_given?
if ::Kernel.block_given?
@target << " ["
_newline
_nested_structures(block)
@@ -236,7 +236,7 @@ module Builder
# #=> <?aaa bbb="ccc"?>
#
def instruct!(directive_tag=:xml, attrs={})
_ensure_no_block block_given?
_ensure_no_block ::Kernel.block_given?
if directive_tag == :xml
a = { :version=>"1.0", :encoding=>"UTF-8" }
attrs = a.merge attrs
@@ -257,7 +257,7 @@ module Builder
# #=> <![CDATA[text to be included in cdata]]>
#
def cdata!(text)
_ensure_no_block block_given?
_ensure_no_block ::Kernel.block_given?
_special("<![CDATA[", "]]>", text, nil)
end
@@ -309,7 +309,7 @@ module Builder
def _attr_value(value)
case value
when Symbol
when ::Symbol
value.to_s
else
_escape_quote(value.to_s)

View File

@@ -1,64 +0,0 @@
# Extensions to +nil+ which allow for more helpful error messages for people who
# are new to Rails.
#
# Ruby raises NoMethodError if you invoke a method on an object that does not
# respond to it:
#
# $ ruby -e nil.destroy
# -e:1: undefined method `destroy' for nil:NilClass (NoMethodError)
#
# With these extensions, if the method belongs to the public interface of the
# classes in NilClass::WHINERS the error message suggests which could be the
# actual intended class:
#
# $ script/runner nil.destroy
# ...
# You might have expected an instance of ActiveRecord::Base.
# ...
#
# NilClass#id exists in Ruby 1.8 (though it is deprecated). Since +id+ is a fundamental
# method of Active Record models NilClass#id is redefined as well to raise a RuntimeError
# and warn the user. She probably wanted a model database identifier and the 4
# returned by the original method could result in obscure bugs.
#
# The flag <tt>config.whiny_nils</tt> determines whether this feature is enabled.
# By default it is on in development and test modes, and it is off in production
# mode.
class NilClass
WHINERS = [::Array]
WHINERS << ::ActiveRecord::Base if defined? ::ActiveRecord
METHOD_CLASS_MAP = Hash.new
WHINERS.each do |klass|
methods = klass.public_instance_methods - public_instance_methods
class_name = klass.name
methods.each { |method| METHOD_CLASS_MAP[method.to_sym] = class_name }
end
# Raises a RuntimeError when you attempt to call +id+ on +nil+.
def id
raise RuntimeError, "Called id for nil, which would mistakenly be 4 -- if you really wanted the id of nil, use object_id", caller
end
private
def method_missing(method, *args, &block)
# Ruby 1.9.2: disallow explicit coercion via method_missing.
if method == :to_ary || method == :to_str
super
elsif klass = METHOD_CLASS_MAP[method]
raise_nil_warning_for klass, method, caller
else
super
end
end
# Raises a NoMethodError when you attempt to call a method on +nil+.
def raise_nil_warning_for(class_name = nil, selector = nil, with_caller = nil)
message = "You have a nil object when you didn't expect it!"
message << "\nYou might have expected an instance of #{class_name}." if class_name
message << "\nThe error occurred while evaluating nil.#{selector}" if selector
raise NoMethodError, message, with_caller || caller
end
end

View File

@@ -0,0 +1,98 @@
require 'abstract_unit'
require 'active_support/concern'
class ConcernTest < ActiveSupport::TestCase
module Baz
extend ActiveSupport::Concern
module ClassMethods
def baz
"baz"
end
def included_ran=(value)
@@included_ran = value
end
def included_ran
@@included_ran
end
end
included do
self.included_ran = true
end
def baz
"baz"
end
end
module Bar
extend ActiveSupport::Concern
include Baz
def bar
"bar"
end
def baz
"bar+" + super
end
end
module Foo
extend ActiveSupport::Concern
include Bar, Baz
end
def setup
@klass = Class.new
end
def test_module_is_included_normally
@klass.send(:include, Baz)
assert_equal "baz", @klass.new.baz
assert @klass.included_modules.include?(ConcernTest::Baz)
end
def test_class_methods_are_extended
@klass.send(:include, Baz)
assert_equal "baz", @klass.baz
assert_equal ConcernTest::Baz::ClassMethods, (class << @klass; self.included_modules; end)[0]
end
def test_included_block_is_ran
@klass.send(:include, Baz)
assert_equal true, @klass.included_ran
end
def test_modules_dependencies_are_met
@klass.send(:include, Bar)
assert_equal "bar", @klass.new.bar
assert_equal "bar+baz", @klass.new.baz
assert_equal "baz", @klass.baz
assert @klass.included_modules.include?(ConcernTest::Bar)
end
def test_dependencies_with_multiple_modules
@klass.send(:include, Foo)
assert_equal [ConcernTest::Foo, ConcernTest::Bar, ConcernTest::Baz], @klass.included_modules[0..2]
end
def test_raise_on_multiple_included_calls
assert_raises(ActiveSupport::Concern::MultipleIncludedBlocks) do
Module.new do
extend ActiveSupport::Concern
included do
end
included do
end
end
end
end
end

View File

@@ -1,50 +0,0 @@
# Stub to enable testing without Active Record
module ActiveRecord
class Base
def save!
end
end
end
require 'abstract_unit'
require 'active_support/whiny_nil'
class WhinyNilTest < Test::Unit::TestCase
def test_unchanged
nil.method_thats_not_in_whiners
rescue NoMethodError => nme
assert(nme.message =~ /nil:NilClass/)
end
def test_active_record
nil.save!
rescue NoMethodError => nme
assert(!(nme.message =~ /nil:NilClass/))
assert_match(/nil\.save!/, nme.message)
end
def test_array
nil.each
rescue NoMethodError => nme
assert(!(nme.message =~ /nil:NilClass/))
assert_match(/nil\.each/, nme.message)
end
def test_id
nil.id
rescue RuntimeError => nme
assert(!(nme.message =~ /nil:NilClass/))
end
def test_no_to_ary_coercion
nil.to_ary
rescue NoMethodError => nme
assert(nme.message =~ /nil:NilClass/)
end
def test_no_to_str_coercion
nil.to_str
rescue NoMethodError => nme
assert(nme.message =~ /nil:NilClass/)
end
end

View File

@@ -5,9 +5,6 @@
# since you don't have to restart the webserver when you make code changes.
config.cache_classes = false
# Log error messages when you accidentally call methods on nil.
config.whiny_nils = true
# Show full error reports and disable caching
config.action_controller.consider_all_requests_local = true
config.action_controller.perform_caching = false

View File

@@ -6,9 +6,6 @@
# and recreated between test runs. Don't rely on the data there!
config.cache_classes = true
# Log error messages when you accidentally call methods on nil.
config.whiny_nils = true
# Show full error reports and disable caching
config.action_controller.consider_all_requests_local = true
config.action_controller.perform_caching = false
@@ -25,4 +22,4 @@ config.action_mailer.delivery_method = :test
# Use SQL instead of Active Record's schema dumper when creating the test database.
# This is necessary if your schema can't be completely dumped by the schema dumper,
# like if you have constraints or database-specific column types
# config.active_record.schema_format = :sql
# config.active_record.schema_format = :sql

View File

@@ -147,7 +147,6 @@ module Rails
initialize_framework_logging
initialize_dependency_mechanism
initialize_whiny_nils
initialize_time_zone
initialize_i18n
@@ -543,12 +542,6 @@ Run `rake gems:install` to install the missing gems.
ActiveSupport::Dependencies.mechanism = configuration.cache_classes ? :require : :load
end
# Loads support for "whiny nil" (noisy warnings when methods are invoked
# on +nil+ values) if Configuration#whiny_nils is true.
def initialize_whiny_nils
require('active_support/whiny_nil') if configuration.whiny_nils
end
# Sets the default value for Time.zone, and turns on ActiveRecord::Base#time_zone_aware_attributes.
# If assigned value cannot be matched to a TimeZone, an exception will be raised.
def initialize_time_zone
@@ -752,10 +745,6 @@ Run `rake gems:install` to install the missing gems.
# The root of the application's views. (Defaults to <tt>app/views</tt>.)
attr_accessor :view_path
# Set to +true+ if you want to be warned (noisily) when you try to invoke
# any method of +nil+. Set to +false+ for the standard Ruby behavior.
attr_accessor :whiny_nils
# The list of plugins to load. If this is set to <tt>nil</tt>, all plugins will
# be loaded. If this is set to <tt>[]</tt>, no plugins will be loaded. Otherwise,
# plugins will be loaded in the order specified.
@@ -870,7 +859,6 @@ Run `rake gems:install` to install the missing gems.
self.preload_frameworks = default_preload_frameworks
self.cache_classes = default_cache_classes
self.dependency_loading = default_dependency_loading
self.whiny_nils = default_whiny_nils
self.plugins = default_plugins
self.plugin_paths = default_plugin_paths
self.plugin_locators = default_plugin_locators
@@ -1067,10 +1055,6 @@ Run `rake gems:install` to install the missing gems.
true
end
def default_whiny_nils
false
end
def default_plugins
nil
end