Commit Graph

96 Commits

Author SHA1 Message Date
Ryuta Kamizono
c81af6ae72 Enable Layout/EmptyLinesAroundAccessModifier cop
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.
2019-06-13 12:00:45 +09:00
Abhay Nikam
00b3b68602 Bump rubocop to 0.71 2019-06-06 15:34:50 +05:30
yuuji.yaginuma
ea5f509643 Change ActionDispatch::Response#content_type returning Content-Type header 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.
39de7fac05/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 ]
2019-06-01 09:20:13 +09:00
Kasper Timm Hansen
563bf5771f
Merge pull request #35793 from jhawthorn/deprecate_layout_absolute_path
Deprecate render layout with an absolute path
2019-03-31 19:05:27 +02:00
John Hawthorn
6841d0cc6d 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.
2019-03-29 11:39:40 -07:00
John Hawthorn
c7820d8124 Introduce Template::File as new render 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.
2019-03-27 15:51:25 -07:00
Koichi ITO
1e1adadb41 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
2019-03-27 07:57:43 +09:00
Aaron Patterson
d4015a7f06
Pass locals in to the template object on construction
This commit ensures that locals are passed in to the template objects
when they are constructed, then removes the `locals=` mutator on the
template object.  This means we don't need to mutate Template objects
with locals in the `decorate` method.
2019-02-25 15:14:53 -08:00
Aaron Patterson
ca5e23ed4d
Templates have one format
Templates only have one format.  Before this commit, templates would be
constructed with a single element array that contained the format.  This
commit eliminates the single element array and just implements a
`format` method.  This saves one array allocation per template.
2019-02-25 13:18:44 -08:00
Aaron Patterson
d0733ba8c0
Move inline rendering content-type test to a controller test
This commit is to remove direct access to the "rendered_format"
attribute on the lookup context.  "rendered_format" is an implementation
detail that we shouldn't test directly.
2019-02-19 13:46:09 -08:00
Aaron Patterson
28f88e0074
Pass source to template handler and deprecate old style handler
This commit passes the mutated source to the template handler as a
parameter and deprecates the old handlers.  Old handlers required that
templates contain a reference to mutated source code, but we would like
to make template objects "read only".  This change lets the template
remain "read only" while still giving template handlers access to the
source code after mutations.
2019-02-01 16:19:53 -08:00
Aaron Patterson
e17fe52e0e
Tighten up the AV::Base constructor
The AV::Base constructor was too complicated, and this commit tightens
up the parameters it will take.  At runtime, AV::Base is most commonly
constructed here:

  94d54fa4ab/actionview/lib/action_view/rendering.rb (L72-L74)

This provides an AV::Renderer instance, a hash of assignments, and a
controller instance.  Since this is the common case for construction, we
should remove logic from the constructor that handles other cases.  This
commit introduces special constructors for those other cases.
Interestingly, most code paths that construct AV::Base "strangely" are
tests.
2019-01-29 15:49:40 -08:00
Ufuk Kayserilioglu
44c07fd0be Add test for Hash-like object being passed to partial locals
The test passes an instance of `ActionController::Parameters` that acts
like a Hash but does not respond to some Hash methods like
`symbolize_keys`.

Moreover, if someone were to call `to_h` on the value it would fail since
the parameter is not permitted. So this is a great way to ensure that the
partial rendering pipeline does not mess with `locals`.
2019-01-24 22:41:19 +02:00
Ufuk Kayserilioglu
d5fcb5b9ad Revert "Allow usage of strings as locals for partial renderer" 2019-01-24 22:15:46 +02:00
Aaron Patterson
3301804ff6
make bot happy 2018-09-24 16:01:34 -07:00
Aaron Patterson
4f96e739c2
Remove deprecated catch-all route in the AV tests
This commit removes a deprecated catch-all route in the AV tests.  It
defines and includes the necessary routes for each test such that we
don't need the catch-all anymore.

This also helps push us toward #33970
2018-09-24 15:39:15 -07:00
Rafael França
ce58a64f19
Merge pull request #30647 from droptheplot/render-partials-string-locals
Allow usage of strings as locals for partial renderer
2018-04-27 19:17:36 -04:00
bogdanvlviv
b41c981bc4
Fix actionview tests execution
On my local environment execution of `cd actionview/ && bin/test` raises error:

```
(snip)
rails/actionview/test/template/render_test.rb:6:in `<top (required)>': superclass mismatch for class TestController (TypeError)
```

In some test files `TestController` inherited from `ActionController::Base`,
but in `test/actionpack/controller/render_test.rb` file `TestController`
inherited from `ApplicationController`.
This produces error `superclass mismatch for class TestController (TypeError)`

Step to reproduce this on any environment:
`cd actionview/ && bin/test test/template/streaming_render_test.rb test/actionpack/controller/render_test.rb`
2018-03-04 18:52:26 +02:00
Ryuta Kamizono
245c1dafa8 Enable Layout/LeadingCommentSpace to not allow cosmetic changes in the future
Follow up of #31432.
2017-12-14 17:30:54 +09:00
Matthew Draper
7d264ba8cd Merge pull request #31005 from shuheiktgw/remove_unnecessary_semicolons
Removed unnecessary semicolons
2017-10-28 22:55:34 +10:30
Sergey Novikov
3b9e3ceff5 Allow usage of strings as locals for partial renderer 2017-09-18 22:12:51 +02:00
Koichi ITO
7c260ae201 Fix RuboCop offenses
And enable `context_dependent` of Style/BracesAroundHashParameters cop.
2017-08-16 17:55:25 +09:00
Kir Shatrov
424117281e Use frozen string literal in actionview/ 2017-07-24 11:53:43 +03:00
Matthew Draper
87b3e226d6 Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
2017-07-02 02:15:17 +09:30
Kir Shatrov
cfade1ec7e Enforce frozen string in Rubocop 2017-07-01 02:11:03 +03:00
bogdanvlviv
40bdbce191
Define path with __dir__
".. with __dir__ we can restore order in the Universe." - by @fxn

Related to 5b8738c2df003a96f0e490c43559747618d10f5f
2017-05-23 00:53:51 +03:00
Akira Matsuda
0157608508 DRY fake models for testing 2017-02-02 16:43:29 +09:00
Akira Matsuda
9360b6be63 class Foo < Struct.new(:x) creates an extra unneeded anonymous class
because Struct.new returns a Class, we just can give it a name and use it directly without inheriting from it
2017-01-13 15:13:47 +09:00
yuuji.yaginuma
75607a67b4 use Gem.win_platform? to check windows Ruby platforms
`Gem.win_platform?` check if it is Windows more accurately.
Ref: https://github.com/ruby/ruby/blob/ruby_2_2/lib/rubygems.rb#L945..L952
2016-11-30 20:57:50 +09:00
Ryuta Kamizono
d99b3848bb Fix require_dependency message format
`depend_on` message format is `"No such file to load -- %s.rb"`.
But `require_dependency` message is missing `.rb` suffix.

```
% git grep -n 'No such file to load'
actionview/test/actionpack/abstract/helper_test.rb:112:        assert_equal "No such file to load -- very_invalid_file_name.rb", e.message
activesupport/lib/active_support/dependencies.rb:245:      def require_dependency(file_name, message = "No such file to load -- %s.rb")
activesupport/lib/active_support/dependencies.rb:333:    def depend_on(file_name, message = "No such file to load -- %s.rb")
```
2016-11-25 06:56:05 +09:00
Rafael Mendonça França
fe1f4b2ad5
Add more rubocop rules about whitespaces 2016-10-29 01:17:49 -02:00
Xavier Noria
56832e791f let Regexp#match? be globally available
Regexp#match? should be considered to be part of the Ruby core library. We are
emulating it for < 2.4, but not having to require the extension is part of the
illusion of the emulation.
2016-10-27 09:13:55 +02:00
Ryuta Kamizono
3464cd5c28 Fix broken comments indentation caused by rubocop auto-correct [ci skip]
All indentation was normalized by rubocop auto-correct at 80e66cc4d90bf8c15d1a5f6e3152e90147f00772.
But comments was still kept absolute position. This commit aligns
comments with method definitions for consistency.
2016-09-14 18:26:32 +09:00
Rafael Mendonça França
55f9b8129a
Add three new rubocop rules
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces

Fix all violations in the repository.
2016-08-16 04:30:11 -03:00
Ryuta Kamizono
7924be5a79 Fix actionview test failure
Caused by #26092.
2016-08-11 14:04:45 +09:00
Ryuta Kamizono
f006de5dc5 Fix broken alignments caused by auto-correct commit 411ccbd
Hash syntax auto-correcting breaks alignments. 411ccbdab2608c62aabdb320d52cb02d446bb39c
2016-08-10 06:36:39 +09:00
Xavier Noria
a9dc45459a code gardening: removes redundant selfs
A few have been left for aesthetic reasons, but have made a pass
and removed most of them.

Note that if the method `foo` returns an array, `foo << 1`
is a regular push, nothing to do with assignments, so
no self required.
2016-08-08 01:12:38 +02:00
Xavier Noria
b326e82dc0 applies remaining conventions across the project 2016-08-06 20:20:22 +02:00
Xavier Noria
80e66cc4d9 normalizes indentation and whitespace across the project 2016-08-06 20:16:27 +02:00
Xavier Noria
411ccbdab2 remove redundant curlies from hash arguments 2016-08-06 19:44:11 +02:00
Xavier Noria
63fff600ac modernizes hash syntax in actionview 2016-08-06 19:36:34 +02:00
Xavier Noria
4b6c68dfb8 applies new string literal convention in actionview/test
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
2016-08-06 18:50:17 +02:00
Xavier Noria
7ebef567ce systematic revision of =~ usage in AV
Where appropriate, prefer the more concise Regexp#match?,
String#include?, String#start_with?, or String#end_with?
2016-07-25 00:31:35 +02:00
Sean Griffin
e6bb1a1b1f Delete bad test
This test was broken by f650e0324207e46ed5240380e60bdf1e2a5023a6. It was
added by https://github.com/rails/rails/pull/17978, and is adequately
tested elsewhere. The reason that this breaks is that
`Controller#process` is not going to set a new response object, and we
now terminate in callbacks if the response has been sent. The only
reason that this test was calling `get` in the first place was because
the controller under test blows up if `request` was `nil`. The point
being that the failure is invalid, and I don't think we need to fix the
test in this location.
2016-06-07 14:36:14 -04:00
Rafael Mendonça França
ce5538be68
Add more test coverage to layouts
[Rafael Mendonça França + Nick Sutterer + thedarkone]
2016-05-20 22:41:32 -03:00
Aaron Patterson
d6bac04692
keep layouts + locals from bloating the cache
Using locals will cause layouts to be cached multiple times in the
template cache.  This commit removes locals from consideration when
looking up the layout.
2016-05-17 11:28:51 -07:00
Aaron Patterson
07fe3569e6
locals can be accessed from templates rendered in the controller 2016-05-17 09:50:51 -07:00
yui-knk
1099329be0 Delete needless require 'active_support/deprecation'
When `require 'active_support/rails'`, 'active_support/deprecation'
is automatically loaded.
2015-10-20 20:02:59 +09:00
Jeremy Daer
565094a8b5 Use Mime[:foo] instead of Mime::Type[:FOO] for back compat
Rails 4.x and earlier didn't support `Mime::Type[:FOO]`, so libraries
that support multiple Rails versions would've had to feature-detect
whether to use `Mime::Type[:FOO]` or `Mime::FOO`.

`Mime[:foo]` has been around for ages to look up registered MIME types
by symbol / extension, though, so libraries and plugins can safely
switch to that without breaking backward- or forward-compatibility.

Note: `Mime::ALL` isn't a real MIME type and isn't registered for lookup
by type or extension, so it's not available as `Mime[:all]`. We use it
internally as a wildcard for `respond_to` negotiation. If you use this
internal constant, continue to reference it with `Mime::ALL`.

Ref. efc6dd550ee49e7e443f9d72785caa0f240def53
2015-10-06 11:29:30 -07:00
Aaron Patterson
4ddbd437c6 render should return a string 2015-10-05 17:33:59 -07:00