Permalink
Browse files

Merge pull request #9258 from wangjohn/blank_argument_errors_in_arel

Raising an error when query methods have blank arguments.
  • Loading branch information...
2 parents 00147c8 + 4033c50 commit 8991083e31c507b2385e8258983261640caa32bf @rafaelfranca rafaelfranca committed Feb 19, 2013
Showing with 34 additions and 9 deletions.
  1. +11 −0 activerecord/CHANGELOG.md
  2. +23 −9 activerecord/lib/active_record/relation/query_methods.rb
View
@@ -1,5 +1,16 @@
## Rails 4.0.0 (unreleased) ##
+* ActiveRecord now raises an error when blank arguments are passed to query
+ methods for which blank arguments do not make sense. This also occurs for
+ nil-like objects in arguments.
+
+ Example:
+
+ Post.limit() # => raises error
+ Post.include([]) # => raises error
+
+ *John Wang*
+
* Simplified type casting code for timezone aware attributes to use the
`in_time_zone` method if it is available. This introduces a subtle change
of behavior when using `Date` instances as they are directly converted to
@@ -108,7 +108,8 @@ def create_with_value # :nodoc:
#
# User.includes(:posts).where('posts.name = ?', 'example').references(:posts)
def includes(*args)
- args.empty? ? self : spawn.includes!(*args)
+ check_empty_arguments("includes", *args)
+ spawn.includes!(*args)
end
def includes!(*args) # :nodoc:
@@ -125,7 +126,8 @@ def includes!(*args) # :nodoc:
# FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" =
# "users"."id"
def eager_load(*args)
- args.blank? ? self : spawn.eager_load!(*args)
+ check_empty_arguments("eager_load", *args)
+ spawn.eager_load!(*args)
end
def eager_load!(*args) # :nodoc:
@@ -138,7 +140,8 @@ def eager_load!(*args) # :nodoc:
# User.preload(:posts)
# => SELECT "posts".* FROM "posts" WHERE "posts"."user_id" IN (1, 2, 3)
def preload(*args)
- args.blank? ? self : spawn.preload!(*args)
+ check_empty_arguments("preload", *args)
+ spawn.preload!(*args)
end
def preload!(*args) # :nodoc:
@@ -155,7 +158,8 @@ def preload!(*args) # :nodoc:
# User.includes(:posts).where("posts.name = 'foo'").references(:posts)
# # => Query now knows the string references posts, so adds a JOIN
def references(*args)
- args.blank? ? self : spawn.references!(*args)
+ check_empty_arguments("references", *args)
+ spawn.references!(*args)
end
def references!(*args) # :nodoc:
@@ -234,7 +238,8 @@ def select!(*fields) # :nodoc:
# User.group('name AS grouped_name, age')
# => [#<User id: 3, name: "Foo", age: 21, ...>, #<User id: 2, name: "Oscar", age: 21, ...>, #<User id: 5, name: "Foo", age: 23, ...>]
def group(*args)
- args.blank? ? self : spawn.group!(*args)
+ check_empty_arguments("group", *args)
+ spawn.group!(*args)
end
def group!(*args) # :nodoc:
@@ -264,7 +269,8 @@ def group!(*args) # :nodoc:
# User.order(:name, email: :desc)
# => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC
def order(*args)
- args.blank? ? self : spawn.order!(*args)
+ check_empty_arguments("order", *args)
+ spawn.order!(*args)
end
def order!(*args) # :nodoc:
@@ -289,7 +295,8 @@ def order!(*args) # :nodoc:
#
# generates a query with 'ORDER BY name ASC, id ASC'.
def reorder(*args)
- args.blank? ? self : spawn.reorder!(*args)
+ check_empty_arguments("reorder", *args)
+ spawn.reorder!(*args)
end
def reorder!(*args) # :nodoc:
@@ -311,7 +318,8 @@ def reorder!(*args) # :nodoc:
# User.joins("LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id")
# => SELECT "users".* FROM "users" LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id
def joins(*args)
- args.compact.blank? ? self : spawn.joins!(*args.flatten)
+ check_empty_arguments("joins", *args)
+ spawn.joins!(*args.flatten)
end
def joins!(*args) # :nodoc:
@@ -475,7 +483,8 @@ def where!(opts = :chain, *rest) # :nodoc:
#
# Order.having('SUM(price) > 30').group('user_id')
def having(opts, *rest)
- opts.blank? ? self : spawn.having!(opts, *rest)
+ check_empty_arguments("having", opts)
route
route Feb 20, 2013 Contributor

@jeremy @rafaelfranca ruby will raise error if we don't pass the first argument, check_empty_arguments will raise confusing error that method must contain arguments if we pass nil as first argument.

rafaelfranca
rafaelfranca Feb 20, 2013 Owner

@route could you show an example of this?

route
route Feb 21, 2013 Contributor

having('') will raise an error that it doesn't have any arguments

+ spawn.having!(opts, *rest)
end
def having!(opts, *rest) # :nodoc:
@@ -912,5 +921,10 @@ def validate_order_args(args)
end
end
+ def check_empty_arguments(method_name, *args)
+ if args.blank?
+ raise ArgumentError, "The method .#{method_name}() must contain arguments."
+ end
+ end
end
end

0 comments on commit 8991083

Please sign in to comment.