This PR removes the deprecation added in #47005 because it is
non-trivial for HAML and Slim to implement error highlighting for
templates. If a template engine doesn't support `translate_location`
then we can fallback to `spot` and the error will be displayed but we
won't be able to automatically underline the column.
Ruby 3.2 significantly changed how instance variables are store.
It now use shapes, and in short, it's important for performance
to define instance variables in a consistent order to limit the
amount of shapes.
Otherwise, the number of shapes will increase past a point where
MRI won't be able to cache instance variable access. The impact
is even more important when YJIT is enabled.
This PR is data driven. I dump the list of Shapes from Shopify's
monolith production environment, and Rails is very present among
the top offenders:
```
Shape Edges Report
-----------------------------------
770 @default_graphql_name
697 @own_fields
661 @to_non_null_type
555 @own_interface_type_memberships
472 @description
389 @errors
348 @oseid
316 @_view_runtime
310 @_db_runtime
292 @visibility
286 @shop
271 @attribute_method_patterns_cache
264 @namespace_for_serializer
254 @locking_column
254 @primary_key
253 @validation_context
244 @quoted_primary_key
238 @access_controls
234 @_trigger_destroy_callback
226 @_trigger_update_callback
224 @finder_needs_type_condition
215 @_committed_already_called
214 @api_type
203 @mutations_before_last_save
202 @access_controls_overrides
201 @options
198 @mutations_from_database
190 @_already_called
183 @name
179 @_request
176 @own_arguments
175 @_assigns
175 @virtual_path
174 @context
173 @_controller
173 @output_buffer
173 @view_flow
172 @_default_form_builder
169 @cache
159 @_touch_record
151 @attribute_names
151 @default_attributes
150 @columns_hash
149 @attribute_types
148 @columns
147 @marked_for_same_origin_verification
146 @schema_loaded
143 @_config
143 @type
141 @column_names
```
All the changes are of similar nature, the goal is to preset the instance
variable to nil when objects are allocated, or when classes are created.
For classes I leverage the `inherited` hook. If the patern becomes common enough
it might make sense to add a helper for this in `ActiveSupport::Concern`.
If other libraries like `haml` don't implement `translate_location` then
they won't get the error highlighting behavior that was implemented to
underline errors coming from templates.
This deprecation is to notify templating engines that this change is
necessary for them to get this highlighting.
We want to use error highlight with eval'd code, specifically ERB
templates.
Previously we could only get the information we needed by setting
`keep_script_lines` to true. In Ruby 3.2 and error_highlight we added
the ability to get this information without setting `keep_script_lines`.
This change implements that new behavior for Rails.
I removed the script line changes to support this in 3.1 because it is
not in any released version.
Ruby change: https://github.com/ruby/ruby/pull/6593
Erorr highlight change: https://github.com/ruby/error_highlight/pull/26
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Ref: ad39d6b
Ensure that all evals include file and line number to identify their
source.
Two of the evals reported by this cop were unneccesary and replaced with
non-eval alternatives: xml is set as a local variable in
Template::Handlers::Builder#call, and instance_eval isn't needed to get
the current binding.
There are additionally 27 offenses in test directories, but since it
seems less important to fix those they are currently ignored.
This is a continuation of #46706 to make sure we don't need to set an
instance variable to `@original_source` for the `compile` method to use.
We can't call `strict_locals!` after encode so we need to set it to a
local variable in `complile`. We changed the `strict_locals!` method to
check `NONE` instead of lazily defining instance variables which let us
simplify `strict_locals?` to return the value of `strict_locals!`. This
simplifies and clarifies the code.
Co-authored-by: Aaron Patterson tenderlove@ruby-lang.org
I do this a lot:
```erb
<%= select :post, :author, authors, required: true %>
```
It doesn't work; the `required` attribute is ignored! Instead, you need to do this:
```erb
<%= select :post, :author, authors, {}, required: true %>
```
It's hard to remember the right API, and it looks to me like a code smell. It looks even smellier when you end up with this:
```erb
<%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
```
Where this would be nicer, but again, the `required` attribute is ignored:
```erb
<%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
```
This PR implements a special handling for `required`, `multiple`, and `size` HTML attributes so that these now do the same thing:
```erb
<%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
<%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
```
ps. as proof I'm not the only person who makes this mistake, one of the tests in the Rails test suite was wrong! The test added in https://github.com/rails/rails/pull/40522 puts the `multiple` attribute in the wrong place and has the wrong assertion as as result. This PR includes a fix for the test.
Moves the part of `compile!` that compiles the template source into it's
own method. We need this for future work in improving exceptions for ERB
templates to pass to ErrorHighlight.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
This change makes date/time options (value, min, max) in `time_field`, `date_field`, `datetime_field`, `week_field`, `month_field` form helpers behave in a unified way.
Since engine initializers run later in the process, we need to run this
initializer earlier than the default.
This ensures they're all registered before the environments are loaded.
`deprecate_constant` will warn whenever the constant is referenced
instead of when the constant is a method receiver, which increases the
likelihood that the warning will be seen. Additionally,
`DeprecatedConstantProxy` prevents using the constant as a superclass,
such as in `class MyClass < SomeDeprecatedConstant`.
This commit adds `ActionView.deprecator` and replaces all usages of
`ActiveSupport::Deprecation.warn` in `actionview/lib` with
`ActionView.deprecator`. This commit also replaces a call to Ruby's
`Module#deprecate_constant` with Rails' `DeprecatedConstantProxy`, so
that its deprecation behavior can be configured using
`ActionView.deprecator`.
Additionally, this commit adds `ActionView.deprecator` to
`Rails.application.deprecators` so that it can be configured via
settings such as `config.active_support.report_deprecations`.
This commit also removes a few defunct `assert_deprecated` calls that
were not failing because they were nested in `assert_raises`, and the
raised error prevented checking the deprecation. (One was mistakenly
kept in d52d773946 when converting
`test_render_file_with_errors` to `test_render_template_with_errors`;
the other two were added in dd9991bac5 but
not removed when the deprecation was completed in
85ecf6e4098601222b604f7c1cbdcb4e49a6d1f0.)
This commit adds `ActionDispatch.deprecator` and replaces all usages of
`ActiveSupport::Deprecation.warn` in `actionpack/lib/action_dispatch`
with `ActionDispatch.deprecator`.
Additionally, this commit adds `ActionDispatch.deprecator` to
`Rails.application.deprecators` so that it can be configured via
settings such as `config.active_support.report_deprecations`.
Currently if you do this:
```ruby
check_box_tag "admin", "1", checked: false
```
It is treated [as truthy](19f9922523/actionview/lib/action_view/helpers/form_tag_helper.rb (L444)), and your checkbox is checked. This can be a bit surprising, particularly because the `FormHelper` version [does support](19f9922523/actionview/lib/action_view/helpers/form_helper.rb (L1285)) a keyword argument.
```ruby
f.check_box "admin", checked: false
```
So this PR updates `check_box_tag` and `radio_button_tag` to support `checked` as a positional or keyword argument, this way you can use the same API in both cases.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
jbuilder is doing some weird things such as instantiatign RenderedTemplate
with a `Hash` instance as a `body`.
So we can't forcibly cast to string in these places.
zzak