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 d52d7739468153bd6cb7c629f60bd5cd7ebea3eb when converting
`test_render_file_with_errors` to `test_render_template_with_errors`;
the other two were added in dd9991bac598bb5da312278a749cf85e19b027cc 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.
We recently let a few very easy to avoid warnings get merged.
The root cause is that locally the test suite doesn't run in
verbose mode unless you explictly pass `-w`.
On CI warnings are enabled, but there is no reason to look at the
build output unless something is failing. And even if one wanted
to do that, that would be particularly work intensive since warnings
may be specific to a Ruby version etc.
Because of this I believe we should:
- Always run the test suite with warnings enabled.
- Raise an error if a warning is unexpected.
We've been using this pattern for a long time at Shopify both in private
and public repositories.
We have access to the path from the backtrace location object. If we
use the path of the ERB as the key, then anytime the ERB changes it'll
just overwrite that template instance in the error handling hash
This commit maps the column information returned from ErrorHighlight in
to column information within the source ERB template. ErrorHighlight
only understands the compiled Ruby code, so this commit adds a small
translation layer that converts the values from ErrorHighlight in to the
right values for the ERB source template
We should get out of the business of parsing backtraces and only use
backtrace locations. Backtrace locations have the file and line number
information baked in, so we don't need to parse things anymore
This commit adds a SyntaxErrorProxy object to active support and wraps
syntax error exceptions with that proxy object. We want to enhance
syntax errors with information about the source location where they
actually happened (normally the backtrace doesn't contain such info).
Rather than mutating the original exception's backtrace, this wraps it
with a proxy object.
Eventually we will implement backtrace_locations on the proxy object so
that the exception handling middleware can be updated to _only_ deal
with backtrace_locations and never deal with raw `backtrace`
