diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 00000000..fb48a77e --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1 @@ +* @github/ruby-architecture-reviewers diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 00000000..2ef4f602 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,73 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +In the interest of fostering an open and welcoming environment, we as +contributors and maintainers pledge to make participation in our project and +our community a harassment-free experience for everyone, regardless of age, body +size, disability, ethnicity, sex characteristics, gender identity and expression, +level of experience, education, socio-economic status, nationality, personal +appearance, race, religion, or sexual identity and orientation. + +## Our Standards + +Examples of behavior that contributes to creating a positive environment +include: + +* Using welcoming and inclusive language +* Being respectful of differing viewpoints and experiences +* Gracefully accepting constructive criticism +* Focusing on what is best for the community +* Showing empathy towards other community members + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery and unwelcome sexual attention or + advances +* Trolling, insulting/derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or electronic + address, without explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Our Responsibilities + +Project maintainers are responsible for clarifying the standards of acceptable +behavior and are expected to take appropriate and fair corrective action in +response to any instances of unacceptable behavior. + +Project maintainers have the right and responsibility to remove, edit, or +reject comments, commits, code, wiki edits, issues, and other contributions +that are not aligned to this Code of Conduct, or to ban temporarily or +permanently any contributor for other behaviors that they deem inappropriate, +threatening, offensive, or harmful. + +## Scope + +This Code of Conduct applies within all project spaces, and it also applies when +an individual is representing the project or its community in public spaces. +Examples of representing a project or community include using an official +project e-mail address, posting via an official social media account, or acting +as an appointed representative at an online or offline event. Representation of +a project may be further defined and clarified by project maintainers. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported by contacting the project team at opensource+rubocop@github.com. All +complaints will be reviewed and investigated and will result in a response that +is deemed necessary and appropriate to the circumstances. The project team is +obligated to maintain confidentiality with regard to the reporter of an incident. +Further details of specific enforcement policies may be posted separately. + +Project maintainers who do not follow or enforce the Code of Conduct in good +faith may face temporary or permanent repercussions as determined by other +members of the project's leadership. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, +available at https://www.contributor-covenant.org/version/1/4/code-of-conduct.html + +[homepage]: https://www.contributor-covenant.org diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..a69b7d28 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,42 @@ +# Contributing + +We welcome your contributions to the project. Thank you! + +Please note that this project is released with a [Contributor Code of Conduct](CODE_OF_CONDUCT.md). By participating in this project you agree to abide by its terms. + + +## What to contribute + +This repository, `rubocop-github`, is part of a broader RuboCop ecosystem. + +If the Cop you would like to propose is **generally applicable outside of GitHub**: + +1. Propose the change upstream in the core open source project (e.g. [`rubocop`](https://github.com/rubocop/rubocop), or [`rubocop-rails`](https://github.com/rubocop/rubocop-rails)), where it will have maximal visibility and discussion/feedback. +1. Patch the change provisionally into GitHub's project(s), for immediate benefit; that can include this repository. +1. ...if the proposal is accepted, remove the patch and pull the updated upstream. +1. ...if the proposal is not accepted, we usually learn something about our proposal, and we then choose whether to maintain the patch ourselves, discard it, or identify a better open-source home for it. + +If the Cop is **only applicable for GitHub**, then this is the right place to propose it. + +## How to contribute a Pull Request + +1. Fork and clone the repository +1. [Build it and make sure the tests pass](README.md#Testing) on your machine +1. Create a new branch: `git checkout -b my-branch-name` +1. Make your change, add tests, and make sure the tests still pass +1. Push to your fork and submit a Pull Request +1. Pat yourself on the back and wait for your pull request to be reviewed and merged. + +## For Maintainers + +### Releasing a new version + +1. Update `rubocop-github.gemspec` with the next version number +1. Update the `CHANGELOG` with changes and contributor +1. Run `bundle` to update gem version contained in the lockfile +1. Make a commit: `Release v{VERSION}` +1. Tag the commit : `git tag v{VERSION}` +1. Create the package: `gem build rubocop-github.gemspec` +1. Push to GitHub: `git push origin && git push origin --tags` +1. Push to Rubygems: `gem push rubocop-github-{VERSION}.gem` +1. Publish a new release on GitHub: https://github.com/github/rubocop-github/releases/new diff --git a/README.md b/README.md index 8796a67d..7ba558fa 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # RuboCop GitHub ![CI](https://github.com/github/rubocop-github/workflows/CI/badge.svg?event=push) -This repository provides recommended RuboCop configuration and additional Cops for use on GitHub open source and internal Ruby projects. +This repository provides recommended RuboCop configuration and additional Cops for use on GitHub open source and internal Ruby projects, and is the home of [GitHub's Ruby Style Guide](./STYLEGUIDE.md). ## Usage diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md index 1e55a3be..758651b6 100644 --- a/STYLEGUIDE.md +++ b/STYLEGUIDE.md @@ -27,6 +27,9 @@ This is GitHub's Ruby Style Guide, inspired by [RuboCop's guide][rubocop-guide]. 1. [Conditional keywords](#conditional-keywords) 2. [Ternary operator](#ternary-operator) 17. [Syntax](#syntax) +18. [Rails](#rails) + 1. [content_for](#content_for) + 2. [Instance Variables in Views](#instance-variables-in-views) ## Layout @@ -34,9 +37,11 @@ This is GitHub's Ruby Style Guide, inspired by [RuboCop's guide][rubocop-guide]. * Use soft-tabs with a two space indent. [[link](#default-indentation)] + * RuboCop rule: Layout/IndentationStyle * Indent `when` with the start of the `case` expression. [[link](#indent-when-as-start-of-case)] + * RuboCop rule: Layout/CaseIndentation ``` ruby # bad @@ -80,10 +85,18 @@ end * Never leave trailing whitespace. [[link](#trailing-whitespace)] + * RuboCop rule: Layout/TrailingWhitespace * Use spaces around operators, after commas, colons and semicolons, around `{` and before `}`. [[link](#spaces-operators)] + * RuboCop rule: Layout/SpaceAroundOperators + * RuboCop rule: Layout/SpaceAfterComma + * RuboCop rule: Layout/SpaceAfterColon + * RuboCop rule: Layout/SpaceBeforeBlockBraces + * RuboCop rule: Layout/SpaceInsideHashLiteralBraces + * RuboCop rule: Style/HashSyntax + * RuboCop rule: Layout/SpaceAroundOperators ``` ruby sum = 1 + 2 @@ -94,6 +107,8 @@ a, b = 1, 2 * No spaces after `(`, `[` or before `]`, `)`. [[link](#no-spaces-braces)] + * RuboCop rule: Layout/SpaceInsideParens + * RuboCop rule: Layout/SpaceInsideReferenceBrackets ``` ruby some(arg).other @@ -102,6 +117,7 @@ some(arg).other * No spaces after `!`. [[link](#no-spaces-bang)] + * RuboCop rule: Layout/SpaceAfterNot ``` ruby !array.include?(element) @@ -111,10 +127,12 @@ some(arg).other * End each file with a [newline](https://github.com/bbatsov/ruby-style-guide#newline-eof). [[link](#newline-eof)] + * RuboCop rule: Layout/TrailingEmptyLines * Use empty lines between `def`s and to break up a method into logical paragraphs. [[link](#empty-lines-def)] + * RuboCop rule: Layout/EmptyLineBetweenDefs ``` ruby def some_method @@ -134,12 +152,14 @@ end * Keep each line of code to a readable length. Unless you have a reason to, keep lines to a maximum of 118 characters. Why 118? That's the width at which the pull request diff UI needs horizontal scrolling (making pull requests harder to review). [[link](#line-length)] + * RuboCop rule: Layout/LineLength ## Classes * Avoid the usage of class (`@@`) variables due to their unusual behavior in inheritance. [[link](#class-variables)] + * RuboCop rule: Style/ClassVars ``` ruby class Parent @@ -164,6 +184,7 @@ Parent.print_class_var # => will print "child" * Use `def self.method` to define singleton methods. This makes the methods more resistant to refactoring changes. [[link](#singleton-methods)] + * RuboCop rule: Style/ClassMethodsDefinitions ``` ruby class TestClass @@ -181,6 +202,7 @@ class TestClass * Avoid `class << self` except when necessary, e.g. single accessors and aliased attributes. [[link](#class-method-definitions)] + * RuboCop rule: Style/ClassMethodsDefinitions ``` ruby class TestClass @@ -214,6 +236,8 @@ end * Indent the `public`, `protected`, and `private` methods as much the method definitions they apply to. Leave one blank line above them. [[link](#access-modifier-identation)] + * RuboCop rule: Layout/AccessModifierIndentation + * RuboCop rule: Layout/EmptyLinesAroundAccessModifier ``` ruby class SomeClass @@ -231,6 +255,7 @@ end * Avoid explicit use of `self` as the recipient of internal class or instance messages unless to specify a method shadowed by a variable. [[link](#self-messages)] + * RuboCop rule: Style/RedundantSelf ``` ruby class SomeClass @@ -248,6 +273,7 @@ end * Prefer `%w` to the literal array syntax when you need an array of strings. [[link](#percent-w)] + * RuboCop rule: Style/WordArray ``` ruby # bad @@ -265,6 +291,7 @@ STATES = %w(draft open closed) * Use symbols instead of strings as hash keys. [[link](#symbols-as-keys)] + * RuboCop rule: Style/StringHashKeys ``` ruby # bad @@ -300,9 +327,10 @@ end Avoid calling `send` and its cousins unless you really need it. Metaprogramming can be extremely powerful, but in most cases you can write code that captures your meaning by being explicit: [[link](#avoid-send)] + * RuboCop rule: Style/Send ``` ruby -# avoid +# avoid unless [:base, :head].include?(base_or_head) raise ArgumentError, "base_or_head must be either :base or :head" end @@ -366,6 +394,7 @@ end Use the Ruby 1.9 syntax for hash literals when all the keys are symbols: [[link](#symbols-as-hash-keys)] +* RuboCop rule: Style/StringHashKeys ``` ruby # bad @@ -396,6 +425,7 @@ link_to("Account", controller: "users", action: "show", id: user) If you have a hash with mixed key types, use the legacy hashrocket style to avoid mixing styles within the same hash: [[link](#consistent-hash-syntax)] +* RuboCop rule: Style/HashSyntax ``` ruby @@ -417,6 +447,7 @@ hsh = { [Keyword arguments](http://magazine.rubyist.net/?Ruby200SpecialEn-kwarg) are recommended but not required when a method's arguments may otherwise be opaque or non-obvious when called. Additionally, prefer them over the old "Hash as pseudo-named args" style from pre-2.0 ruby. [[link](#keyword-arguments)] +* RuboCop rule: Style/OptionalBooleanParameter So instead of this: @@ -444,21 +475,26 @@ remove_member(user, skip_membership_check: true) * Use `snake_case` for methods and variables. [[link](#snake-case-methods-vars)] + * RuboCop rule: Naming/SnakeCase + * RuboCop rule: Naming/VariableName * Use `CamelCase` for classes and modules. (Keep acronyms like HTTP, RFC, XML uppercase.) [[link](#camelcase-classes-modules)] + * RuboCop rule: Naming/ClassAndModuleCamelCase * Use `SCREAMING_SNAKE_CASE` for other constants. [[link](#screaming-snake-case-constants)] + * RuboCop rule: Naming/ConstantName * The names of predicate methods (methods that return a boolean value) should end in a question mark. (i.e. `Array#empty?`). [[link](#bool-methods-qmark)] + * RuboCop rule: Naming/PredicateName * The names of potentially "dangerous" methods (i.e. methods that modify `self` or the arguments, `exit!`, etc.) should end with an exclamation mark. Bang methods - should only exist if a non-bang counterpart (method name which does NOT end with !) + should only exist if a non-bang counterpart (method name which does NOT end with !) also exists. [[link](#dangerous-method-bang)] @@ -466,6 +502,7 @@ remove_member(user, skip_membership_check: true) * Use `%w` freely. [[link](#use-percent-w-freely)] + * RuboCop rule: Style/WordArray ``` ruby STATES = %w(draft open closed) @@ -474,6 +511,7 @@ STATES = %w(draft open closed) * Use `%()` for single-line strings which require both interpolation and embedded double-quotes. For multi-line strings, prefer heredocs. [[link](#percent-parens-single-line)] + * RuboCop rule: Style/BarePercentLiterals ``` ruby # bad (no interpolation needed) @@ -494,6 +532,7 @@ STATES = %w(draft open closed) * Use `%r` only for regular expressions matching *more than* one '/' character. [[link](#percent-r-regular-expressions)] + * RuboCop rule: Style/RegexpLiteral ``` ruby # bad @@ -512,7 +551,7 @@ STATES = %w(draft open closed) * Avoid using $1-9 as it can be hard to track what they contain. Named groups can be used instead. [[link](#capture-with-named-groups)] - + * RuboCop rule: Lint/MixedRegexpCaptureTypes ``` ruby # bad /(regexp)/ =~ string @@ -571,6 +610,7 @@ documentation about the libraries that the current file uses. * Prefer string interpolation instead of string concatenation: [[link](#string-interpolation)] + * RuboCop rule: Style/StringConcatenation ``` ruby # bad @@ -584,6 +624,7 @@ email_with_name = "#{user.name} <#{user.email}>" will always work without a delimiter change, and `'` is a lot more common than `"` in string literals. [[link](#double-quotes)] + * RuboCop rule: Style/StringLiterals ``` ruby # bad @@ -615,6 +656,7 @@ end * Use `def` with parentheses when there are arguments. Omit the parentheses when the method doesn't accept any arguments. [[link](#method-parens-when-arguments)] + * RuboCop rule: Style/DefWithParentheses ``` ruby def some_method @@ -632,9 +674,11 @@ end always use parentheses in the method invocation. For example, write `f((3 + 2) + 1)`. [[link](#parens-no-spaces)] + * RuboCop rule: Style/MethodCallWithArgsParentheses * Never put a space between a method name and the opening parenthesis. [[link](#no-spaces-method-parens)] + * RuboCop rule: Style/ParenthesesAsGroupedExpression ``` ruby # bad @@ -650,6 +694,7 @@ f(3 + 2) + 1 * Never use `then` for multi-line `if/unless`. [[link](#no-then-for-multi-line-if-unless)] + * RuboCop rule: Style/MultilineIfThen ``` ruby # bad @@ -665,10 +710,12 @@ end * The `and` and `or` keywords are banned. It's just not worth it. Always use `&&` and `||` instead. [[link](#no-and-or-or)] + * RuboCop rule: Style/AndOr * Favor modifier `if/unless` usage when you have a single-line body. [[link](#favor-modifier-if-unless)] + * RuboCop rule: Style/MultilineTernaryOperator ``` ruby # bad @@ -682,6 +729,7 @@ do_something if some_condition * Never use `unless` with `else`. Rewrite these with the positive case first. [[link](#no-else-with-unless)] + * RuboCop rule: Style/UnlessElse ``` ruby # bad @@ -701,6 +749,7 @@ end * Don't use parentheses around the condition of an `if/unless/while`. [[link](#no-parens-if-unless-while)] + * RuboCop rule: Style/ParenthesesAroundCondition ``` ruby # bad @@ -720,6 +769,7 @@ end trivial. However, do use the ternary operator(`?:`) over `if/then/else/end` constructs for single line conditionals. [[link](#trivial-ternary)] + * RuboCop rule: Style/MultilineTernaryOperator ``` ruby # bad @@ -731,11 +781,13 @@ result = some_condition ? something : something_else * Avoid multi-line `?:` (the ternary operator), use `if/unless` instead. [[link](#no-multiline-ternary)] + * RuboCop rule: Style/MultilineTernaryOperator * Use one expression per branch in a ternary operator. This also means that ternary operators must not be nested. Prefer `if/else` constructs in these cases. [[link](#one-expression-per-branch)] + * RuboCop rule: Style/NestedTernaryOperator ``` ruby # bad @@ -757,6 +809,7 @@ end doesn't introduce a new scope (unlike `each`) and variables defined in its block will be visible outside it. [[link](#avoid-for)] + * RuboCop rule: Style/For ``` ruby arr = [1, 2, 3] @@ -776,6 +829,7 @@ arr.each { |elem| puts elem } definitions" (e.g. in Rakefiles and certain DSLs). Avoid `do...end` when chaining. [[link](#squiggly-braces)] + * RuboCop rule: Style/BlockDelimiters ``` ruby names = ["Bozhidar", "Steve", "Sarah"] @@ -798,11 +852,12 @@ end.map { |name| name.upcase } ``` * Some will argue that multiline chaining would look OK with the use of `{...}`, - but they should ask themselves: is this code really readable and can't the block's + but they should ask themselves: is this code really readable and can't the block's contents be extracted into nifty methods? * Avoid `return` where not required. [[link](#avoid-return)] + * RuboCop rule: Style/RedundantReturn ``` ruby # bad @@ -818,6 +873,7 @@ end * Use spaces around the `=` operator when assigning default values to method parameters: [[link](#spaces-around-equals)] + * RuboCop rule: Style/SpaceAroundEqualsInParameterDefault ``` ruby # bad @@ -850,6 +906,7 @@ if (v = next_value) == "hello" ... * Use `||=` freely to initialize variables. [[link](#memoize-away)] + * RuboCop rule: Style/OrAssignment ``` ruby # set name to Bozhidar, only if it's nil or false @@ -859,6 +916,7 @@ name ||= "Bozhidar" * Don't use `||=` to initialize boolean variables. (Consider what would happen if the current value happened to be `false`.) [[link](#no-memoization-for-boolean)] + * RuboCop rule: Style/OrAssignment ``` ruby # bad - would set enabled to true even if it was false @@ -873,9 +931,11 @@ enabled = true if enabled.nil? one-liner scripts is discouraged. Prefer long form versions such as `$PROGRAM_NAME`. [[link](#no-cryptic-vars)] + * RuboCop rule: Style/SpecialGlobalVars * Use `_` for unused block parameters. [[link](#underscore-unused-vars)] + * RuboCop rule: Lint/UnusedBlockArgument ``` ruby # bad @@ -890,7 +950,133 @@ result = hash.map { |_, v| v + 1 } For example, `String === "hi"` is true and `"hi" === String` is false. Instead, use `is_a?` or `kind_of?` if you must. [[link](#type-checking-is-a-kind-of)] + * RuboCop rule: Style/CaseEquality Refactoring is even better. It's worth looking hard at any code that explicitly checks types. +## Rails + +### content_for + +Limit usage of `content_for` helper. The use of `content_for` is the same as setting an instance variable plus `capture`. + +``` erb +<% content_for :foo do %> + Hello +<% end %> +``` + +Is effectively the same as + +``` erb +<% @foo_content = capture do %> + Hello +<% end %> +``` + +See "Instance Variables in Views" below. + +#### Common Anti-patterns + +**Using `content_for` within the same template to capture data.** + +Instead, just use `capture`. + +``` erb + +<% content_for :page do %> + Hello +<% end %> +<% if foo? %> +
+ <%= yield :page %> +
+<% else %> + <%= yield :page %> +<% end %> +``` + +Simply capture and use a local variable since the result is only needed in this template. + +``` erb + +<% page = capture do %> + Hello +<% end %> +<% if foo? %> +
+ <%= page %> +
+<% else %> + <%= page %> +<% end %> +``` + +**Using `content_for` to pass content to a subtemplate.** + +Instead, `render layout:` with a block. + +``` erb + +<% content_for :page do %> + Hello +<% end %> +<%= render partial: "page" %> + +
+ <%= yield :page %> +
+``` + +Pass the content in a block directly to the `render` function. + +``` erb + +<%= render layout: "page" do %> + Hello +<% end %> + +
+ <%= yield %> +
+``` + +### Instance Variables in Views + +In general, passing data between templates with instance variables is discouraged. This even applies from controllers to templates, not just between partials. + +`:locals` can be used to pass data from a controller just like partials. + +``` ruby +def show + render "blob/show", locals: { + :repository => current_repository, + :commit => current_commit, + :blob => current_blob + } +end +``` + +Rails implicit renders are also discouraged. + +Always explicitly render templates with a full directory path. This makes template callers easier to trace. You can find all the callers of `"app/view/site/hompage.html.erb"` with a simple project search for `"site/homepage"`. + +``` ruby +def homepage + render "site/homepage" +end +``` + +#### Exceptions + +There are some known edge cases where you might be forced to use instance variables. In these cases, its okay to do so. + +##### Legacy templates + +If you need to call a subview that expects an instance variable be set. If possible consider refactoring the subview to accept a local instead. + +##### Layouts + +Unfortunately the only way to get data into a layout template is with instance variables. You can't explicitly pass locals to them. + [rubocop-guide]: https://github.com/rubocop-hq/ruby-style-guide diff --git a/config/default.yml b/config/default.yml index 7aef6c8d..6b46b224 100644 --- a/config/default.yml +++ b/config/default.yml @@ -182,16 +182,14 @@ Layout/HeredocIndentation: Enabled: false Layout/IndentationConsistency: - Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#indentation Layout/IndentationStyle: - Enabled: true EnforcedStyle: spaces IndentationWidth: 2 StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#default-indentation Layout/IndentationWidth: - Enabled: true Width: 2 StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#default-indentation @@ -262,11 +260,9 @@ Layout/SingleLineBlockChain: Enabled: false Layout/SpaceAfterColon: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators Layout/SpaceAfterComma: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators Layout/SpaceAfterMethodName: @@ -276,18 +272,16 @@ Layout/SpaceAfterNot: Enabled: true Layout/SpaceAfterSemicolon: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators Layout/SpaceAroundBlockParameters: Enabled: true Layout/SpaceAroundEqualsInParameterDefault: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-around-equals Layout/SpaceAroundKeyword: - Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators Layout/SpaceAroundMethodCallOperator: Enabled: false @@ -317,7 +311,6 @@ Layout/SpaceInLambdaLiteral: Enabled: false Layout/SpaceInsideArrayLiteralBrackets: - Enabled: true EnforcedStyle: no_space StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-spaces-braces @@ -349,7 +342,6 @@ Layout/TrailingEmptyLines: Enabled: true Layout/TrailingWhitespace: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#trailing-whitespace Lint/AmbiguousAssignment: @@ -703,7 +695,7 @@ Lint/UnreachableLoop: # TODO: Enable this since it's in the styleguide. Lint/UnusedBlockArgument: Enabled: false - StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-around-equals + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#underscore-unused-vars Lint/UnusedMethodArgument: Enabled: false @@ -1102,7 +1094,6 @@ Style/DateTime: Enabled: false Style/DefWithParentheses: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#method-parens-when-arguments Style/Dir: @@ -1193,7 +1184,6 @@ Style/FloatDivision: Enabled: false Style/For: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#avoid-for Style/FormatString: @@ -1230,7 +1220,6 @@ Style/HashLikeCase: Enabled: false Style/HashSyntax: - Enabled: true EnforcedStyle: ruby19_no_mixed_keys StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#symbols-as-keys @@ -1334,7 +1323,6 @@ Style/MultilineIfModifier: Enabled: false Style/MultilineIfThen: - Enabled: true StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-then-for-multi-line-if-unless Style/MultilineInPatternThen: @@ -1601,8 +1589,8 @@ Style/StringHashKeys: Enabled: false Style/StringLiterals: - Enabled: true EnforcedStyle: double_quotes + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#double-quotes Style/StringLiteralsInInterpolation: Enabled: false diff --git a/config/rails.yml b/config/rails.yml index 3114c3cf..81ef8a4c 100644 --- a/config/rails.yml +++ b/config/rails.yml @@ -2,9 +2,6 @@ require: - rubocop-github-rails - rubocop-rails -GitHub/RailsApplicationRecord: - Enabled: true - GitHub/RailsControllerRenderActionSymbol: Enabled: true @@ -17,9 +14,6 @@ GitHub/RailsControllerRenderPathsExist: GitHub/RailsControllerRenderShorthand: Enabled: true -GitHub/RailsRenderInline: - Enabled: true - GitHub/RailsRenderObjectCollection: Enabled: false @@ -94,7 +88,7 @@ Rails/ApplicationMailer: Enabled: false Rails/ApplicationRecord: - Enabled: false + Enabled: true Rails/ArelStar: Enabled: false @@ -307,7 +301,7 @@ Rails/RelativeDateConstant: Enabled: false Rails/RenderInline: - Enabled: false + Enabled: true Rails/RenderPlainText: Enabled: false diff --git a/config/rails_cops.yml b/config/rails_cops.yml index f8f817e1..1443e4ba 100644 --- a/config/rails_cops.yml +++ b/config/rails_cops.yml @@ -1,6 +1,3 @@ -GitHub/RailsApplicationRecord: - Enabled: pending - GitHub/RailsControllerRenderActionSymbol: Enabled: pending Include: diff --git a/guides/rails-render-inline.md b/guides/rails-render-inline.md deleted file mode 100644 index 8b5fa9b9..00000000 --- a/guides/rails-render-inline.md +++ /dev/null @@ -1,27 +0,0 @@ -# GitHub/RailsRenderInline - -tldr; Do not use `render inline:`. - -## Rendering plain text - -``` ruby -render inline: "Just plain text" # bad -``` - -The `inline:` option is often misused when plain text is being returned. Instead use `render plain: "Just plain text"`. - -## Rendering a dynamic ERB string - -String `#{}` interpolation is often misused with `render inline:` instead of using `<%= %>` interpolation. This will lead to a memory leak where an ERB method will be compiled and cached for each invocation of `render inline:`. - -``` ruby -render inline: "Hello #{@name}" # bad -``` - -## Rendering static ERB strings - -``` ruby -render inline: "Hello <%= @name %>" # bad -``` - -If you are passing a static ERB string to `render inline:`, this string is best moved to a `.erb` template under `app/views`. Template files are able to be precompiled at boot time. diff --git a/lib/rubocop/cop/github/rails_application_record.rb b/lib/rubocop/cop/github/rails_application_record.rb deleted file mode 100644 index 1ec30cf9..00000000 --- a/lib/rubocop/cop/github/rails_application_record.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require "rubocop" - -module RuboCop - module Cop - module GitHub - class RailsApplicationRecord < Base - MSG = "Models should subclass from ApplicationRecord" - - def_node_matcher :active_record_base_const?, <<-PATTERN - (const (const nil? :ActiveRecord) :Base) - PATTERN - - def_node_matcher :application_record_const?, <<-PATTERN - (const nil? :ApplicationRecord) - PATTERN - - def on_class(node) - klass, superclass, _ = *node - - if active_record_base_const?(superclass) && !(application_record_const?(klass)) - add_offense(superclass) - end - end - end - end - end -end diff --git a/lib/rubocop/cop/github/rails_render_inline.rb b/lib/rubocop/cop/github/rails_render_inline.rb deleted file mode 100644 index 8f76fb2f..00000000 --- a/lib/rubocop/cop/github/rails_render_inline.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require "rubocop" - -module RuboCop - module Cop - module GitHub - class RailsRenderInline < Base - MSG = "Avoid `render inline:`" - - def_node_matcher :render_with_options?, <<-PATTERN - (send nil? {:render :render_to_string} (hash $...)) - PATTERN - - def_node_matcher :inline_key?, <<-PATTERN - (pair (sym :inline) $_) - PATTERN - - def on_send(node) - if option_pairs = render_with_options?(node) - if option_pairs.detect { |pair| inline_key?(pair) } - add_offense(node) - end - end - end - end - end - end -end diff --git a/test/test_rails_application_record.rb b/test/test_rails_application_record.rb deleted file mode 100644 index 7ac77945..00000000 --- a/test/test_rails_application_record.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require_relative "./cop_test" -require "minitest/autorun" -require "rubocop/cop/github/rails_application_record" - -class TestRailsApplicationRecord < CopTest - def cop_class - RuboCop::Cop::GitHub::RailsApplicationRecord - end - - def test_good_model - offenses = investigate(cop, <<-RUBY) - class Repository < ApplicationRecord - end - RUBY - - assert_empty offenses.map(&:message) - end - - def test_application_record_model - offenses = investigate(cop, <<-RUBY) - class ApplicationRecord < ActiveRecord::Base - end - RUBY - - assert_empty offenses.map(&:message) - end - - def test_bad_model - offenses = investigate(cop, <<-RUBY) - class Repository < ActiveRecord::Base - end - RUBY - - assert_equal 1, offenses.count - assert_equal "Models should subclass from ApplicationRecord", offenses.first.message - end -end diff --git a/test/test_rails_render_inline.rb b/test/test_rails_render_inline.rb deleted file mode 100644 index 53d5cadd..00000000 --- a/test/test_rails_render_inline.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -require_relative "./cop_test" -require "minitest/autorun" -require "rubocop/cop/github/rails_render_inline" - -class TestRailsRenderInline < CopTest - def cop_class - RuboCop::Cop::GitHub::RailsRenderInline - end - - def test_render_string_no_offense - offenses = investigate cop, <<-RUBY, "app/controllers/foo_controller.rb" - class FooController < ActionController::Base - def index - render template: "index" - end - - def show - render template: "show.html.erb", locals: { foo: @foo } - end - end - RUBY - - assert_equal 0, offenses.count - end - - def test_render_inline_offense - offenses = investigate cop, <<-RUBY, "app/controllers/products_controller.rb" - class ProductsController < ActionController::Base - def index - render inline: "<% products.each do |p| %>

<%= p.name %>

<% end %>" - end - end - RUBY - - assert_equal 1, offenses.count - assert_equal "Avoid `render inline:`", offenses[0].message - end - - def test_render_status_with_inline_offense - offenses = investigate cop, <<-RUBY, "app/controllers/products_controller.rb" - class ProductsController < ActionController::Base - def index - render status: 200, inline: "<% products.each do |p| %>

<%= p.name %>

<% end %>" - end - end - RUBY - - assert_equal 1, offenses.count - assert_equal "Avoid `render inline:`", offenses[0].message - end - - def test_erb_render_inline_offense - offenses = erb_investigate cop, <<-ERB, "app/views/products/index.html.erb" - <%= render inline: template %> - ERB - - assert_equal 1, offenses.count - assert_equal "Avoid `render inline:`", offenses[0].message - end -end