| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
| |
The source_extract method will return nil when it can't find the file name in
the backtrace, methods that consume this method expect an array and the nil ends
up causing type errors down the road like it happened here: #36341. This
patch refactors the source_extract method so that it returns an empty
array instead of nil when it can't find the source code.
Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
|
|\
| |
| | |
Fix TranslationHelper#translate handling of Hash defaults
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
It is sometimes expected of the `translate` methods to return a Hash,
for instance it's the case of the `number.format` key.
As such users might need to specify a Hash default, e.g.
`translate(:'some.format', default: { separator: '.', delimiter: ',' })`.
This works as expected with the `I18n.translate` methods,
however `TranslationHelper#translate` apply `Array()` on the default value.
As a result the default value end up as `[:separator, '.', :delimiter, ',']`.
|
|/ |
|
|\
| |
| | |
Fix programmatic clicks with data-remote
|
| | |
|
|\ \
| | |
| | | |
Introduce ActionView::Component
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Co-authored-by: Natasha Umer <natashau@github.com>
Co-authored-by: Aaron Patterson <tenderlove@github.com>
Co-authored-by: Shawn Allen <shawnbot@github.com>
Co-authored-by: Emily Plummer <emplums@github.com>
Co-authored-by: Diana Mounter <broccolini@github.com>
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Co-authored-by: Nathan Herald <myobie@github.com>
Co-authored-by: Zaid Zawaideh <zawaideh@github.com>
Co-authored-by: Zach Ahn <engineering@zachahn.com>
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).
Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).
That cop and enforced style will reduce the our code review cost.
|
|\ \ \
| |_|/
|/| | |
Run actionview tests in parallel
|
| | | |
|
| |/
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We shouldn't modify fixtures (or any files which are checked-in). It
prevents us from parallelizing, and probably has other issues.
We could fix these tests by copying the file to a tmpdir and modifying
it there, but I don't think they are testing anything useful anymore.
Re-initializing a resolver isn't representative of "uncached" rendering
(either in dev-mode or using lookup_context.disable_cache).
|
|/ |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
as it is
Since #35709, `Response#conten_type` returns only MIME type correctly.
It is a documented behavior that this method only returns MIME type, so
this change seems appropriate.
https://github.com/rails/rails/blob/39de7fac0507070e3c5f8b33fbad6fced84d97ed/actionpack/lib/action_dispatch/http/response.rb#L245-L249
But unfortunately, some users expect this method to return all
Content-Type that does not contain charset. This seems to be breaking
changes.
We can change this behavior with the deprecate cycle.
But, in that case, a method needs that include Content-Type with
additional parameters. And that method name is probably the
`content_type` seems to properly.
So I changed the new behavior to more appropriate `media_type` method.
And `Response#content_type` changed (as the method name) to return Content-Type
header as it is.
Fixes #35709.
[Rafael Mendonça França & Yuuji Yaginuma ]
|
| |
|
|
|
|
|
| |
The `GC.start` was added at b29e893, but the finalizer has been removed
at 7d0ce78 in #35036.
|
|
|
|
|
|
|
|
|
| |
The templates rendered in RenderTestCases tests will be cached by the
resolvers unexpectedly. And this will break other tests when executed in
certain order. (See https://github.com/rails/rails/issues/36154 for more
detail)
So to fix this issue, we just need to clear the caches on all resolvers.
|
|\
| |
| | |
Model error as object
|
| |
| |
| |
| | |
Revert some tests to ensure back compatibility
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Performance cops will be extracted from RuboCop to RuboCop Performance
when next RuboCop 0.68 will be released.
https://github.com/rubocop-hq/rubocop/issues/5977
RuboCop 0.67 is its transition period.
Since rails/rails repository uses Performance cops, This PR added
rubocop-performance gem to Gemfile.
And this PR fixes some offenses using the following auto-correct.
```console
% bundle exec rubocop -a
Offenses:
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:212:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator =
> should be surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:239:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator => should be
surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
actionview/test/template/resolver_shared_tests.rb:1:1: C: [Corrected]
Style/FrozenStringLiteralComment: Missing magic comment #
frozen_string_literal: true.
module ResolverSharedTests
^
actionview/test/template/resolver_shared_tests.rb:10:33: C: [Corrected]
Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in
default value assignment.
def with_file(filename, source="File at #{filename}")
^
actionview/test/template/resolver_shared_tests.rb:106:5: C: [Corrected]
Rails/RefuteMethods: Prefer assert_not_same over refute_same.
refute_same a, b
^^^^^^^^^^^
2760 files inspected, 5 offenses detected, 5 offenses corrected
```
|
| | |
|
| |
| |
| |
| |
| |
| | |
This adds a bit of complexity, but is necessary for now to avoid holding
extra copies of templates which are resolved from ActionView::Digestor
after disabling cache on the lookup context.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously it's possible to have multiple copies of the "same" Template.
For example, if index.html.erb is found both the :en and :fr locale, it
will return a different Template object for each. The same can happen
with formats, variants, and handlers.
This commit de-duplicates templates, there will now only be one template
per file/virtual_path/locals tuple.
We need to consider virtual_path because both `render "index"`, and
`render "index.html"` can both find the same file but will have
different virtual_paths. IMO this is rare and should be
deprecated/removed, but it exists now so we need to consider it in order
to cache correctly.
This commit introduces a new UnboundTemplate class, which represents a
template with unknown locals. Template objects can be built from it by
using `#with_locals`. Currently, this is just a convenience around
caching templates, but I hope it's a helpful concept that could have
more utility in the future.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We didn't previously have many tests directly against the
OptimizedFileSystemResolver or FileSystemResolver, though usually
failures would be exposed through other tests.
It's easier to test some specifics of the behaviour with unit tests.
This also lets us test FileSystemResolver (non-optimized) which I don't
think previously had much testing (other than from classses inheriting
it).
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously, we would discard the template source after rendering, if we
had a virtual path, in hopes that the virtual path would let us find our
same template again going through the Resolver.
Previously we discarded the source as an optimization, to avoid keeping
it around in memory. By instead just reading the file every time source
is called, as FileTemplate does, this is unnecessary.
|
| |
| |
| |
| |
| |
| | |
- After https://github.com/rails/rails/pull/35408 and
https://github.com/rails/rails/pull/35406, the `formats` and
`variants` methods are deprecated in favor of `format` and `variant`.
|
| | |
|
|\ \
| | |
| | | |
Fix partial caching ignore repeated items issue
|
| | |
| | |
| | |
| | |
| | |
| | | |
This is because we only use hash to maintain the result. So when the key
are the same, the result would be skipped. The solution is to maintain
an array for tracking every item's position to restructure the result.
|
| | | |
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously, when using `render file:`, it was possible to render files
not only at an absolute path or relative to the current directory, but
relative to ANY view paths. This was probably done for absolutely
maximum compatibility when addressing CVE-2016-0752, but I think is
unlikely to be used in practice.
Tihs commit removes the ability to `render file:` with a path relative
to a non-fallback view path.
Make FallbackResolver.new private
To ensure nobody is making FallbackResolvers other than "/" and "".
Make reject_files_external_... no-op for fallbacks
Because there are only two values used for path: "" and "/", and
File.join("", "") == File.join("/", "") == "/", this method was only
testing that the absolute paths started at "/" (which of course all do).
This commit doesn't change any behaviour, but it makes it explicit that
the FallbackFileSystemResolver works this way.
Remove outside_app_allowed argument
Deprecate find_all_anywhere
This is now equivalent to find_all
Remove outside_app argument
Deprecate find_file for find
Both LookupContext#find_file and PathSet#find_file are now equivalent to
their respective #find methods.
|
| | |
|
|\ \
| |/
|/| |
Deprecate render layout with an absolute path
|
| |
| |
| |
| |
| |
| |
| | |
This has similar problems to render file:.
I've never seen this used, and believe it's a relic from when all
templates could be rendered from an absolute path.
|
|\ \
| |/
|/| |
RFC: Introduce Template::File
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The previous behaviour of render file: was essentially the same as
render template:, except that templates can be specified as an absolute
path on the filesystem.
This makes sense for historic reasons, but now render file: is almost
exclusively used to render raw files (not .erb) like public/404.html. In
addition to complicating the code in template/resolver.rb, I think the
current behaviour is surprising to developers.
This commit deprecates the existing "lookup a template from anywhere"
behaviour and replaces it with "render this file exactly as it is on
disk". Handlers will no longer be used (it will render the same as if
the :raw handler was used), but formats (.html, .xml, etc) will still be
detected (and will default to :plain).
The existing render file: behaviour was the path through which Rails
apps were vulnerable in the recent CVE-2019-5418. Although the
vulnerability has been patched in a fully backwards-compatible way, I
think it's a strong hint that we should drop the existing
previously-vulnerable behaviour if it isn't a benefit to developers.
|
| | |
|
|\ \
| |/
|/|
| |
| | |
erose/better-error-reporting-for-syntax-errors-in-templates
Display a more helpful error message when an ERB template has a Ruby syntax error.
|
| | |
|
|\ \
| | |
| | | |
Bump RuboCop to 0.66.0
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
### Summary
RuboCop 0.66.0 has been released.
https://github.com/rubocop-hq/rubocop/releases/tag/v0.66.0
And rubocop-0-66 channel is available in Code Climate.
https://github.com/codeclimate/codeclimate/releases/tag/v0.84.0
RuboCop 0.66.0 fixed the false negative to indentation for
modifier. And this PR applied the auto-correction fixed by it.
https://github.com/rubocop-hq/rubocop/pull/6792
In addtion, this PR is also updating the following 4 gems that
RuboCop depends on.
- Update Psych gem ... https://github.com/rubocop-hq/rubocop/pull/6766
- Update Parser gem to 2.6.2.0 that supports Ruby 2.5.5 and 2.6.2 ...
https://github.com/whitequark/parser/blob/v2.6.2.0/CHANGELOG.md#changelog
- Remove powerpack gem ... https://github.com/rubocop-hq/rubocop/pull/6806
- Update unicode-display_width gem ... https://github.com/rubocop-hq/rubocop/pull/6813
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Custom glob patterns tie the implementation (Using Dir.glob) to the API
we provide.
It also doesn't really work. extract_handler_and_format_and_variant
expects the handler, format, and variant to be at the end of the
template path, and in the same order as they are in the default pattern.
This deprecates specifying a custom path for FileSystemResolver and
removes the pattern argument of OptimizedFileSystemResolver#initialize,
which does not work with a custom pattern.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Many tests were using `render file:`, but were only testing the
behaviour of `render template:` (file: just allows more paths/ is less
secure then template:).
The reason for so many `render file:` is probably that they were the old
default.
This commit replaces `render file:` with `render template:` anywhere the
test wasn't specifically interested in using `render file:`.
|
|\ \
| | |
| | | |
Validate types assigned to LookupContext#formats=
|
| | | |
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | | |
This is a developer quality of life improvement, to ensure that unknown
formats aren't assigned (which it would previously accept, but wouldn't
work 100% correctly due to caching).
|
| | |
| | |
| | |
| | |
| | | |
Having a format listed twice had no effect. This is mostly helpful to
avoid an extra format when assigning [:html, "*/*"]
|
| | |
| | |
| | |
| | |
| | | |
This also removes the mutation we were performing on the values being
passed in.
|