diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..165ff874 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @github/rubocop-reviewers diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..8afc1973 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,25 @@ +--- +version: 2 +updates: +- package-ecosystem: bundler + directory: "/" + versioning-strategy: increase + schedule: + interval: weekly + open-pull-requests-limit: 10 + allow: + - dependency-type: "all" + groups: + bundler-rubocop: + patterns: + - "rubocop*" + bundler-all: + update-types: + - minor + - patch + exclude-patterns: + - "rubocop*" +- package-ecosystem: github-actions + directory: "/" + schedule: + interval: weekly diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 00000000..86059b7c --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,42 @@ +name: build + +on: + push: + branches: + - main + pull_request: + branches: + - main + workflow_call: + +permissions: + contents: read + +jobs: + build: + name: build + + strategy: + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} + + steps: + - name: checkout + uses: actions/checkout@v4 + + - uses: ruby/setup-ruby@13e7a03dc3ac6c3798f4570bfead2aed4d96abfb # pin@v1.244.0 + with: + bundler-cache: true + + - name: build + run: | + GEM_NAME=$(ls | grep gemspec | cut -d. -f1) + echo "Attempting to build gem $GEM_NAME..." + gem build $GEM_NAME + if [ $? -eq 0 ]; then + echo "Gem built successfully!" + else + echo "Gem build failed!" + exit 1 + fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml deleted file mode 100644 index 9a950db5..00000000 --- a/.github/workflows/ci.yml +++ /dev/null @@ -1,21 +0,0 @@ -name: CI - -on: [push, pull_request] - -jobs: - test: - strategy: - fail-fast: false - matrix: - ruby_version: ["2.7", "3.0", "3.1"] - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - ruby-version: ${{ matrix.ruby_version }} - - name: Install dependencies - run: bundle install --jobs 4 --retry 3 - - name: Run tests - run: bundle exec rake diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..24ca871b --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,26 @@ +name: lint + +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + +jobs: + lint: + name: lint + runs-on: ubuntu-latest + + steps: + - name: checkout + uses: actions/checkout@v4 + + - uses: ruby/setup-ruby@13e7a03dc3ac6c3798f4570bfead2aed4d96abfb # pin@v1.244.0 + with: + bundler-cache: true + + - name: lint + run: script/lint diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..84778eda --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,64 @@ +name: release + +on: + workflow_dispatch: + push: + branches: + - main + paths: + - lib/version.rb + +permissions: + contents: write + packages: write + id-token: write + attestations: write + +jobs: + release: + runs-on: ubuntu-latest + + steps: + - name: checkout + uses: actions/checkout@v4 + + - uses: ruby/setup-ruby@13e7a03dc3ac6c3798f4570bfead2aed4d96abfb # pin@v1.244.0 + with: + bundler-cache: true + + - name: lint + run: script/lint + + - name: test + run: script/test + + - name: set GEM_NAME from gemspec + run: echo "GEM_NAME=$(ls | grep gemspec | cut -d. -f1)" >> $GITHUB_ENV + + # builds the gem and saves the version to GITHUB_ENV + - name: build + run: echo "GEM_VERSION=$(gem build ${{ env.GEM_NAME }}.gemspec 2>&1 | grep Version | cut -d':' -f 2 | tr -d " \t\n\r")" >> $GITHUB_ENV + + - uses: actions/attest-build-provenance@db473fddc028af60658334401dc6fa3ffd8669fd # pin@v2 + with: + subject-path: "${{ env.GEM_NAME }}-${{ env.GEM_VERSION }}.gem" + + - name: publish to GitHub packages + run: | + export OWNER=$( echo ${{ github.repository }} | cut -d "/" -f 1 ) + GEM_HOST_API_KEY=${{ secrets.GITHUB_TOKEN }} gem push --KEY github --host https://rubygems.pkg.github.com/${OWNER} ${{ env.GEM_NAME }}-${{ env.GEM_VERSION }}.gem + + - name: release + uses: ncipollo/release-action@440c8c1cb0ed28b9f43e4d1d670870f059653174 # pin@v1.16.0 + with: + artifacts: "${{ env.GEM_NAME }}-${{ env.GEM_VERSION }}.gem" + tag: "v${{ env.GEM_VERSION }}" + generateReleaseNotes: true + + - name: publish to RubyGems + run: | + mkdir -p ~/.gem + echo -e "---\n:rubygems_api_key: ${{ secrets.RUBYGEMS_API_KEY }}" > ~/.gem/credentials + chmod 0600 ~/.gem/credentials + gem push ${{ env.GEM_NAME }}-${{ env.GEM_VERSION }}.gem + rm ~/.gem/credentials diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..af449019 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,31 @@ +name: test + +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + ruby_version: ["3.1", "3.2", "3.3", "3.4"] + + steps: + - uses: actions/checkout@v4 + + - name: Update .ruby-version with matrix value + run: echo "${{ matrix.ruby_version }}" >| .ruby-version + + - uses: ruby/setup-ruby@13e7a03dc3ac6c3798f4570bfead2aed4d96abfb # pin@v1.244.0 + with: + bundler-cache: true + + - name: test + run: script/test diff --git a/.gitignore b/.gitignore index fb280015..35b4f5bf 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ spec/reports test/tmp test/version_tmp tmp +bin/ # YARD artifacts .yardoc diff --git a/.rubocop.yml b/.rubocop.yml index 9c49771d..50ed10da 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,6 @@ inherit_from: - ./config/default.yml + - ./config/rails.yml Naming/FileName: Enabled: true @@ -7,3 +8,7 @@ Naming/FileName: - "rubocop-github.gemspec" - "lib/rubocop-github.rb" - "lib/rubocop-github-rails.rb" + +Gemspec/DevelopmentDependencies: + Enabled: true + EnforcedStyle: gemspec diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 00000000..4d9d11cf --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +3.4.2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b4ce534..f89036a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,30 @@ # rubocop-github -## Unreleased +## v0.25.0 + +- Read the automatic release notes on [the /releases page for this gem](https://github.com/github/rubocop-github/releases). +- Updated related gems +- Specify plugin class names for included rubocop plugins + +## v0.24.0 + +- Read the automatic release notes on [the /releases page for this gem](https://github.com/github/rubocop-github/releases). + +## v0.23.0 + +- Read the automatic release notes on [the /releases page for this gem](https://github.com/github/rubocop-github/releases). + +## v0.22.0 + +- Read the automatic release notes on [the /releases page for this gem](https://github.com/github/rubocop-github/releases). + +## v0.21.0 + +- Added new GitHub/AvoidObjectSendWithDynamicMethod cop to discourage use of methods like Object#send + +## v0.20.0 + +- Updated minimum dependencies for "rubocop" (`>= 1.37`), "rubocop-performance" (`>= 1.15`), and "rubocop-rails", (`>= 2.17`). ## v0.19.0 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..e5ef656b --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,56 @@ +# 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 + +### Updating Rubocop Dependencies + +Rubocop regularly releases new versions with new cops. We want to keep up to date with the latest Rubocop releases, and keep these rules and styleguide in sync to reduce burden on consumers of this gem. + +- Run `bundle update rubocop rubocop-performance rubocop-rails` to update the dependencies within this repository. Major updates will require updating the `.gemspec` file because of the pinned version constraints. +- Run `bundle exec rubocop`, and copy the output of newly introduced rules into `config/default_pending.yml` and `config/rails_pending.yml`. They should look like this: + + ```sh + Lint/DuplicateMagicComment: # new in 1.37 + Enabled: true + Style/OperatorMethodCall: # new in 1.37 + Enabled: true + Style/RedundantStringEscape: # new in 1.37 + Enabled: true + ``` + +- Run `bundle exec rubocop` again to ensure that it runs cleanly without any pending cops. Also run `bundle exec rake` to run the tests. +- Work through the pending cops, and copy them to `config/{default,rails}.yml` with an explicity `Enabled: true` or `Enabled: false` depending on your decision as to whether they should be part of our standard ruleset. + +### Releasing a new version + +1. Update [`lib/version.rb`](lib/version.rb) with the next version number +2. Update the `CHANGELOG` with changes and contributor +3. Run `bundle install` to update gem version contained in the lockfile +4. Commit your changes and open a pull request +5. When the pull request is approved and merged into `main`, the [`.github/workflows/release.yml`](.github/workflows/release.yml) workflow will automatically run to release the new version to RubyGems and GitHub Packages 🎉. diff --git a/Gemfile.lock b/Gemfile.lock index e597b511..d4d29779 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,87 +1,123 @@ PATH remote: . specs: - rubocop-github (0.19.0) - rubocop (>= 1.0.0) - rubocop-performance - rubocop-rails + rubocop-github (0.25.0) + rubocop (>= 1.72) + rubocop-performance (>= 1.24) + rubocop-rails (>= 2.23) GEM remote: https://rubygems.org/ specs: - actionview (7.0.3) - activesupport (= 7.0.3) + actionview (7.2.2.1) + activesupport (= 7.2.2.1) builder (~> 3.1) - erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.1, >= 1.2.0) - activesupport (7.0.3) - concurrent-ruby (~> 1.0, >= 1.0.2) + erubi (~> 1.11) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + activesupport (7.2.2.1) + base64 + benchmark (>= 0.3) + bigdecimal + concurrent-ruby (~> 1.0, >= 1.3.1) + connection_pool (>= 2.2.5) + drb i18n (>= 1.6, < 2) + logger (>= 1.4.2) minitest (>= 5.1) - tzinfo (~> 2.0) - ast (2.4.2) - builder (3.2.4) - concurrent-ruby (1.1.10) + securerandom (>= 0.3) + tzinfo (~> 2.0, >= 2.0.5) + ast (2.4.3) + base64 (0.2.0) + benchmark (0.4.0) + bigdecimal (3.1.9) + builder (3.3.0) + concurrent-ruby (1.3.5) + connection_pool (2.5.3) crass (1.0.6) - erubi (1.10.0) - i18n (1.10.0) + drb (2.2.3) + erubi (1.13.1) + i18n (1.14.7) concurrent-ruby (~> 1.0) - loofah (2.18.0) + json (2.12.2) + language_server-protocol (3.17.0.5) + lint_roller (1.1.0) + logger (1.7.0) + loofah (2.24.0) crass (~> 1.0.2) - nokogiri (>= 1.5.9) - minitest (5.16.1) - nokogiri (1.13.6-arm64-darwin) + nokogiri (>= 1.12.0) + mini_portile2 (2.8.8) + minitest (5.25.5) + nokogiri (1.18.8) + mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.13.6-x86_64-darwin) + nokogiri (1.18.8-arm64-darwin) racc (~> 1.4) - parallel (1.22.1) - parser (3.1.2.0) + nokogiri (1.18.8-x86_64-darwin) + racc (~> 1.4) + nokogiri (1.18.8-x86_64-linux-gnu) + racc (~> 1.4) + parallel (1.27.0) + parser (3.3.8.0) ast (~> 2.4.1) - racc (1.6.0) - rack (2.2.3.1) - rails-dom-testing (2.0.3) - activesupport (>= 4.2.0) + racc + prism (1.4.0) + racc (1.8.1) + rack (3.1.15) + rails-dom-testing (2.2.0) + activesupport (>= 5.0.0) + minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.4.3) - loofah (~> 2.3) + rails-html-sanitizer (1.6.2) + loofah (~> 2.21) + nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) rainbow (3.1.1) - rake (13.0.6) - regexp_parser (2.5.0) - rexml (3.2.5) - rubocop (1.31.0) + rake (13.2.1) + regexp_parser (2.10.0) + rubocop (1.75.7) + json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) parallel (~> 1.10) - parser (>= 3.1.0.0) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.18.0, < 2.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.44.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.18.0) - parser (>= 3.1.1.0) - rubocop-performance (1.14.2) - rubocop (>= 1.7.0, < 2.0) - rubocop-ast (>= 0.4.0) - rubocop-rails (2.15.1) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.44.1) + parser (>= 3.3.7.2) + prism (~> 1.4) + rubocop-performance (1.25.0) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rails (2.32.0) activesupport (>= 4.2.0) + lint_roller (~> 1.1) rack (>= 1.1) - rubocop (>= 1.7.0, < 2.0) - ruby-progressbar (1.11.0) - tzinfo (2.0.4) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.44.0, < 2.0) + ruby-progressbar (1.13.0) + securerandom (0.4.1) + tzinfo (2.0.6) concurrent-ruby (~> 1.0) - unicode-display_width (2.2.0) + unicode-display_width (3.1.4) + unicode-emoji (~> 4.0, >= 4.0.4) + unicode-emoji (4.0.4) PLATFORMS arm64-darwin-21 + ruby x86_64-darwin-19 x86_64-darwin-20 + x86_64-linux DEPENDENCIES - actionview + actionview (~> 7.2.2.1) minitest rake rubocop-github! BUNDLED WITH - 2.2.33 + 2.6.5 diff --git a/README.md b/README.md index 8796a67d..ed631b24 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,11 @@ -# RuboCop GitHub ![CI](https://github.com/github/rubocop-github/workflows/CI/badge.svg?event=push) +# RuboCop GitHub -This repository provides recommended RuboCop configuration and additional Cops for use on GitHub open source and internal Ruby projects. +[![test](https://github.com/github/rubocop-github/actions/workflows/test.yml/badge.svg)](https://github.com/github/rubocop-github/actions/workflows/test.yml) +[![build](https://github.com/github/rubocop-github/actions/workflows/build.yml/badge.svg)](https://github.com/github/rubocop-github/actions/workflows/build.yml) +[![lint](https://github.com/github/rubocop-github/actions/workflows/lint.yml/badge.svg)](https://github.com/github/rubocop-github/actions/workflows/lint.yml) +[![release](https://github.com/github/rubocop-github/actions/workflows/release.yml/badge.svg)](https://github.com/github/rubocop-github/actions/workflows/release.yml) + +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 @@ -16,10 +21,10 @@ Inherit all of the stylistic rules and cops through an inheritance declaration i ```yaml # .rubocop.yml - inherit_from: + inherit_gem: rubocop-github: - - config/default.yml # generic Ruby rules and cops - - config/rails.yml # Rails-specific rules and cops + - config/default.yml # generic Ruby rules and cops + - config/rails.yml # Rails-specific rules and cops ``` Alternatively, only require the additional custom cops in your `.rubocop.yml` without inheriting/enabling the other stylistic rules: diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md index 1e55a3be..31f33664 100644 --- a/STYLEGUIDE.md +++ b/STYLEGUIDE.md @@ -3,6 +3,7 @@ This is GitHub's Ruby Style Guide, inspired by [RuboCop's guide][rubocop-guide]. ## Table of Contents + 1. [Layout](#layout) 1. [Indentation](#indentation) 2. [Inline](#inline) @@ -27,6 +28,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 +38,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 +86,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 +108,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 +118,7 @@ some(arg).other * No spaces after `!`. [[link](#no-spaces-bang)] + * RuboCop rule: Layout/SpaceAfterNot ``` ruby !array.include?(element) @@ -111,10 +128,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 +153,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 +185,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 +203,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 +237,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 +256,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 +274,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 +292,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 +328,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 +395,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 +426,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 +448,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 +476,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 +503,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 +512,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 +533,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 +552,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 +611,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 +625,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 +657,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 +675,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: Lint/ParenthesesAsGroupedExpression ``` ruby # bad @@ -650,6 +695,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 +711,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 +730,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 +750,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 @@ -714,12 +764,29 @@ if x > 10 end ``` +* Don't use `unless` with a negated condition. + [[link](#no-unless-negation)] + * RuboCop rule: Style/NegatedUnless + +```ruby +# bad +unless !condition? + do_something +end + +# good +if condition? + do_something +end +``` + ### Ternary operator * Avoid the ternary operator (`?:`) except in cases where all expressions are extremely 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 +798,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 +826,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 +846,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 +869,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 +890,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 +923,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 +933,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 +948,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 +967,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 52a0655e..ff7eec81 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1,6 +1,12 @@ +inherit_from: + - ./default_pending.yml + require: - rubocop-github - - rubocop-performance + +plugins: + - rubocop-performance: + plugin_class_name: RuboCop::Performance::Plugin Bundler/DuplicatedGem: Enabled: true @@ -41,6 +47,9 @@ Gemspec/RequiredRubyVersion: Gemspec/RubyVersionGlobalsUsage: Enabled: false +GitHub/AvoidObjectSendWithDynamicMethod: + Enabled: true + GitHub/InsecureHashAlgorithm: Enabled: true @@ -66,7 +75,9 @@ Layout/BlockEndNewline: Enabled: true Layout/CaseIndentation: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#indent-when-as-start-of-case + EnforcedStyle: end Layout/ClassStructure: Enabled: false @@ -180,16 +191,16 @@ 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 Layout/InitialIndentation: Enabled: true @@ -211,6 +222,7 @@ Layout/LineEndStringConcatenationIndentation: Layout/LineLength: Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#line-length Layout/MultilineArrayBraceLayout: Enabled: false @@ -258,10 +270,10 @@ 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: Enabled: true @@ -270,22 +282,25 @@ 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 Layout/SpaceAroundOperators: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators + Exclude: + - "**/*.erb" Layout/SpaceBeforeBlockBraces: Enabled: true @@ -309,8 +324,8 @@ 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 Layout/SpaceInsideArrayPercentLiteral: Enabled: true @@ -319,7 +334,8 @@ Layout/SpaceInsideBlockBraces: Enabled: true Layout/SpaceInsideHashLiteralBraces: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators Layout/SpaceInsideParens: Enabled: true @@ -340,7 +356,7 @@ Layout/TrailingEmptyLines: Enabled: true Layout/TrailingWhitespace: - Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#trailing-whitespace Lint/AmbiguousAssignment: Enabled: false @@ -444,6 +460,15 @@ Lint/EmptyConditionalBody: Lint/EmptyEnsure: Enabled: true +Lint/EmptyExpression: + Enabled: false + +Lint/EmptyFile: + Enabled: false + +Lint/EmptyWhen: + Enabled: false + Lint/EmptyInterpolation: Enabled: true @@ -547,7 +572,9 @@ Lint/OutOfRangeRegexpRef: Enabled: false Lint/ParenthesesAsGroupedExpression: - Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-spaces-method-parens + Exclude: + - "**/*.erb" Lint/PercentStringArray: Enabled: false @@ -597,6 +624,9 @@ Lint/RegexpAsCondition: Lint/RequireParentheses: Enabled: true +Lint/RequireRangeParentheses: + Enabled: false + Lint/RequireRelativeSelfPath: Enabled: false @@ -678,8 +708,10 @@ Lint/UnreachableCode: Lint/UnreachableLoop: Enabled: false +# TODO: Enable this since it's in the styleguide. Lint/UnusedBlockArgument: Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#underscore-unused-vars Lint/UnusedMethodArgument: Enabled: false @@ -977,7 +1009,8 @@ Style/Alias: Enabled: false Style/AndOr: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-and-or-or Style/ArgumentsForwarding: Enabled: false @@ -1014,6 +1047,7 @@ Style/BlockDelimiters: Style/CaseEquality: Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#type-checking-is-a-kind-of Style/CaseLikeIf: Enabled: false @@ -1034,10 +1068,12 @@ Style/ClassMethods: Enabled: true Style/ClassMethodsDefinitions: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#classes Style/ClassVars: Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#class-variables Style/CollectionCompact: Enabled: false @@ -1076,7 +1112,7 @@ 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: Enabled: false @@ -1114,6 +1150,9 @@ Style/EmptyCaseCondition: Style/EmptyElse: Enabled: false +Style/EmptyHeredoc: + Enabled: false + Style/EmptyLambdaParameter: Enabled: false @@ -1163,7 +1202,7 @@ Style/FloatDivision: Enabled: false Style/For: - Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#avoid-for Style/FormatString: Enabled: false @@ -1199,8 +1238,8 @@ 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 Style/HashTransformKeys: Enabled: false @@ -1256,6 +1295,9 @@ Style/LambdaCall: Style/LineEndConcatenation: Enabled: false +Style/MagicCommentFormat: + Enabled: false + Style/MapCompactWithConditionalBlock: Enabled: false @@ -1299,7 +1341,7 @@ 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: Enabled: false @@ -1329,7 +1371,7 @@ Style/NegatedIfElseCondition: Enabled: false Style/NegatedUnless: - Enabled: false + Enabled: true Style/NegatedWhile: Enabled: false @@ -1344,7 +1386,10 @@ Style/NestedParenthesizedCalls: Enabled: false Style/NestedTernaryOperator: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#one-expression-per-branch + Exclude: + - "**/*.erb" Style/Next: Enabled: false @@ -1395,7 +1440,10 @@ Style/OptionalBooleanParameter: Enabled: false Style/OrAssignment: - Enabled: false + Enabled: true + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#memoize-away + Exclude: + - "**/*.erb" Style/ParallelAssignment: Enabled: false @@ -1475,8 +1523,10 @@ Style/RedundantRegexpCharacterClass: Style/RedundantRegexpEscape: Enabled: false +# TODO: Enable this since it's in the styleguide. Style/RedundantReturn: Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#avoid-return Style/RedundantSelf: Enabled: false @@ -1563,8 +1613,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 @@ -1626,8 +1676,10 @@ Style/TrailingUnderscoreVariable: Style/TrivialAccessors: Enabled: false +# TODO: Enable this since it's in the styleguide. Style/UnlessElse: Enabled: false + StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-else-with-unless Style/UnlessLogicalOperators: Enabled: false diff --git a/config/default_pending.yml b/config/default_pending.yml new file mode 100644 index 00000000..5ae412d3 --- /dev/null +++ b/config/default_pending.yml @@ -0,0 +1,116 @@ +Lint/DuplicateMagicComment: # new in 1.37 + Enabled: false +Style/OperatorMethodCall: # new in 1.37 + Enabled: false +Style/RedundantStringEscape: # new in 1.37 + Enabled: false +Gemspec/DevelopmentDependencies: # new in 1.44 + Enabled: false +Lint/DuplicateMatchPattern: # new in 1.50 + Enabled: false +Lint/ItWithoutArgumentsInBlock: # new in 1.59 + Enabled: false +Lint/LiteralAssignmentInCondition: # new in 1.58 + Enabled: false +Lint/MixedCaseRange: # new in 1.53 + Enabled: false +Lint/RedundantRegexpQuantifiers: # new in 1.53 + Enabled: false +Lint/UselessRescue: # new in 1.43 + Enabled: false +Metrics/CollectionLiteralLength: # new in 1.47 + Enabled: false +Style/ArrayIntersect: # new in 1.40 + Enabled: false +Style/ComparableClamp: # new in 1.44 + Enabled: false +Style/ConcatArrayLiterals: # new in 1.41 + Enabled: false +Style/DataInheritance: # new in 1.49 + Enabled: false +Style/DirEmpty: # new in 1.48 + Enabled: false +Style/ExactRegexpMatch: # new in 1.51 + Enabled: false +Style/FileEmpty: # new in 1.48 + Enabled: false +Style/MapIntoArray: # new in 1.63 + Enabled: false +Style/MapToSet: # new in 1.42 + Enabled: false +Style/MinMaxComparison: # new in 1.42 + Enabled: false +Style/RedundantArrayConstructor: # new in 1.52 + Enabled: false +Style/RedundantConstantBase: # new in 1.40 + Enabled: false +Style/RedundantCurrentDirectoryInPath: # new in 1.53 + Enabled: false +Style/RedundantDoubleSplatHashBraces: # new in 1.41 + Enabled: false +Style/RedundantEach: # new in 1.38 + Enabled: false +Style/RedundantFilterChain: # new in 1.52 + Enabled: false +Style/RedundantHeredocDelimiterQuotes: # new in 1.45 + Enabled: false +Style/RedundantLineContinuation: # new in 1.49 + Enabled: false +Style/RedundantRegexpArgument: # new in 1.53 + Enabled: false +Style/RedundantRegexpConstructor: # new in 1.52 + Enabled: false +Style/ReturnNilInPredicateMethodDefinition: # new in 1.53 + Enabled: false +Style/SingleLineDoEndBlock: # new in 1.57 + Enabled: false +Style/SuperWithArgsParentheses: # new in 1.58 + Enabled: false +Style/YAMLFileRead: # new in 1.53 + Enabled: false +Performance/MapMethodChain: # new in 1.19 + Enabled: false +Gemspec/AddRuntimeDependency: # new in 1.65 + Enabled: false +Lint/ConstantReassignment: # new in 1.70 + Enabled: false +Lint/DuplicateSetElement: # new in 1.67 + Enabled: false +Lint/HashNewWithKeywordArgumentsAsDefault: # new in 1.69 + Enabled: false +Lint/NumericOperationWithConstantResult: # new in 1.69 + Enabled: false +Lint/SharedMutableDefault: # new in 1.70 + Enabled: false +Lint/UnescapedBracketInRegexp: # new in 1.68 + Enabled: false +Lint/UselessDefined: # new in 1.69 + Enabled: false +Lint/UselessNumericOperation: # new in 1.66 + Enabled: false +Style/AmbiguousEndlessMethodDefinition: # new in 1.68 + Enabled: false +Style/BitwisePredicate: # new in 1.68 + Enabled: false +Style/CombinableDefined: # new in 1.68 + Enabled: false +Style/DigChain: # new in 1.69 + Enabled: false +Style/FileNull: # new in 1.69 + Enabled: false +Style/FileTouch: # new in 1.69 + Enabled: false +Style/ItAssignment: # new in 1.70 + Enabled: false +Style/KeywordArgumentsMerging: # new in 1.68 + Enabled: false +Style/RedundantInterpolationUnfreeze: # new in 1.66 + Enabled: false +Style/SafeNavigationChainLength: # new in 1.68 + Enabled: false +Style/SendWithLiteralMethodName: # new in 1.64 + Enabled: false +Style/SuperArguments: # new in 1.64 + Enabled: false +Performance/StringBytesize: # new in 1.23 + Enabled: false diff --git a/config/rails.yml b/config/rails.yml index e414a00a..cba38c11 100644 --- a/config/rails.yml +++ b/config/rails.yml @@ -1,9 +1,12 @@ +inherit_from: + - ./rails_pending.yml + require: - rubocop-github-rails - - rubocop-rails -GitHub/RailsApplicationRecord: - Enabled: true +plugins: + - rubocop-rails: + plugin_class_name: RuboCop::Rails::Plugin GitHub/RailsControllerRenderActionSymbol: Enabled: true @@ -17,9 +20,6 @@ GitHub/RailsControllerRenderPathsExist: GitHub/RailsControllerRenderShorthand: Enabled: true -GitHub/RailsRenderInline: - Enabled: true - GitHub/RailsRenderObjectCollection: Enabled: false @@ -34,27 +34,27 @@ GitHub/RailsViewRenderShorthand: Layout/BlockAlignment: Exclude: - - app/views/**/*.erb + - "**/*.erb" Layout/IndentationWidth: Exclude: - - app/views/**/*.erb + - "**/*.erb" Layout/InitialIndentation: Exclude: - - app/views/**/*.erb + - "**/*.erb" Layout/SpaceInsideParens: Exclude: - - app/views/**/*.erb + - "**/*.erb" Layout/TrailingEmptyLines: Exclude: - - app/views/**/*.erb + - "**/*.erb" Layout/TrailingWhitespace: Exclude: - - app/views/**/*.erb + - "**/*.erb" Lint/UselessAccessModifier: ContextCreatingMethods: @@ -94,7 +94,7 @@ Rails/ApplicationMailer: Enabled: false Rails/ApplicationRecord: - Enabled: false + Enabled: true Rails/ArelStar: Enabled: false @@ -307,7 +307,7 @@ Rails/RelativeDateConstant: Enabled: false Rails/RenderInline: - Enabled: false + Enabled: true Rails/RenderPlainText: Enabled: false @@ -399,16 +399,16 @@ Rails/WhereNot: Style/For: Exclude: - - app/views/**/*.erb + - "**/*.erb" Style/OneLineConditional: Exclude: - - app/views/**/*.erb + - "**/*.erb" Style/Semicolon: Exclude: - - app/views/**/*.erb + - "**/*.erb" Style/StringLiterals: Exclude: - - app/views/**/*.erb + - "**/*.erb" diff --git a/config/rails_cops.yml b/config/rails_cops.yml index ef3d9a4f..d0d0a34c 100644 --- a/config/rails_cops.yml +++ b/config/rails_cops.yml @@ -1,6 +1,3 @@ -GitHub/RailsApplicationRecord: - Enabled: pending - GitHub/RailsControllerRenderActionSymbol: Enabled: pending Include: @@ -8,7 +5,7 @@ GitHub/RailsControllerRenderActionSymbol: GitHub/RailsControllerRenderLiteral: Enabled: pending - StyleGuide: https://github.com/github/rubocop-github/blob/master/guides/rails-render-literal.md + StyleGuide: https://github.com/github/rubocop-github/blob/main/guides/rails-render-literal.md Include: - 'app/controllers/**/*.rb' @@ -21,25 +18,16 @@ GitHub/RailsControllerRenderPathsExist: GitHub/RailsControllerRenderShorthand: Enabled: pending - StyleGuide: https://github.com/github/rubocop-github/blob/master/guides/rails-controller-render-shorthand.md + StyleGuide: https://github.com/github/rubocop-github/blob/main/guides/rails-controller-render-shorthand.md Include: - 'app/controllers/**/*.rb' -GitHub/RailsRenderInline: - Enabled: pending - StyleGuide: https://github.com/github/rubocop-github/blob/master/guides/rails-controller-render-inline.md - Include: - - 'app/controllers/**/*.rb' - - 'app/helpers/**/*.rb' - - 'app/view_models/**/*.rb' - - 'app/views/**/*.erb' - GitHub/RailsRenderObjectCollection: Enabled: pending GitHub/RailsViewRenderLiteral: Enabled: pending - StyleGuide: https://github.com/github/rubocop-github/blob/master/guides/rails-render-literal.md + StyleGuide: https://github.com/github/rubocop-github/blob/main/guides/rails-render-literal.md Include: - 'app/helpers/**/*.rb' - 'app/view_models/**/*.rb' diff --git a/config/rails_pending.yml b/config/rails_pending.yml new file mode 100644 index 00000000..941a743c --- /dev/null +++ b/config/rails_pending.yml @@ -0,0 +1,102 @@ +Rails/ActionControllerFlashBeforeRender: # new in 2.16 + Enabled: false +Rails/ActionOrder: # new in 2.17 + Enabled: false +Rails/ActiveSupportOnLoad: # new in 2.16 + Enabled: false +Rails/FreezeTime: # new in 2.16 + Enabled: false +Rails/IgnoredColumnsAssignment: # new in 2.17 + Enabled: false +Rails/RootPathnameMethods: # new in 2.16 + Enabled: false +Rails/ToSWithArgument: # new in 2.16 + Enabled: false +Rails/TopLevelHashWithIndifferentAccess: # new in 2.16 + Enabled: false +Rails/WhereMissing: # new in 2.16 + Enabled: false +Rails/WhereNotWithMultipleConditions: # new in 2.17 + Enabled: false +Gemspec/DevelopmentDependencies: # new in 1.44 + Enabled: false +Lint/DuplicateMatchPattern: # new in 1.50 + Enabled: false +Lint/ItWithoutArgumentsInBlock: # new in 1.59 + Enabled: false +Lint/LiteralAssignmentInCondition: # new in 1.58 + Enabled: false +Lint/MixedCaseRange: # new in 1.53 + Enabled: false +Lint/RedundantRegexpQuantifiers: # new in 1.53 + Enabled: false +Lint/UselessRescue: # new in 1.43 + Enabled: false +Metrics/CollectionLiteralLength: # new in 1.47 + Enabled: false +Style/ArrayIntersect: # new in 1.40 + Enabled: false +Style/ComparableClamp: # new in 1.44 + Enabled: false +Style/ConcatArrayLiterals: # new in 1.41 + Enabled: false +Style/DataInheritance: # new in 1.49 + Enabled: false +Style/DirEmpty: # new in 1.48 + Enabled: false +Style/ExactRegexpMatch: # new in 1.51 + Enabled: false +Style/FileEmpty: # new in 1.48 + Enabled: false +Style/MapIntoArray: # new in 1.63 + Enabled: false +Style/MapToSet: # new in 1.42 + Enabled: false +Style/MinMaxComparison: # new in 1.42 + Enabled: false +Style/RedundantArrayConstructor: # new in 1.52 + Enabled: false +Style/RedundantConstantBase: # new in 1.40 + Enabled: false +Style/RedundantCurrentDirectoryInPath: # new in 1.53 + Enabled: false +Style/RedundantDoubleSplatHashBraces: # new in 1.41 + Enabled: false +Style/RedundantEach: # new in 1.38 + Enabled: false +Style/RedundantFilterChain: # new in 1.52 + Enabled: false +Style/RedundantHeredocDelimiterQuotes: # new in 1.45 + Enabled: false +Style/RedundantLineContinuation: # new in 1.49 + Enabled: false +Style/RedundantRegexpArgument: # new in 1.53 + Enabled: false +Style/RedundantRegexpConstructor: # new in 1.52 + Enabled: false +Style/ReturnNilInPredicateMethodDefinition: # new in 1.53 + Enabled: false +Style/SingleLineDoEndBlock: # new in 1.57 + Enabled: false +Style/SuperWithArgsParentheses: # new in 1.58 + Enabled: false +Style/YAMLFileRead: # new in 1.53 + Enabled: false +Performance/MapMethodChain: # new in 1.19 + Enabled: false +Rails/DangerousColumnNames: # new in 2.21 + Enabled: false +Rails/EnvLocal: # new in 2.22 + Enabled: false +Rails/RedundantActiveRecordAllMethod: # new in 2.21 + Enabled: false +Rails/ResponseParsedBody: # new in 2.18 + Enabled: false +Rails/ThreeStateBooleanColumn: # new in 2.19 + Enabled: false +Rails/UnusedRenderContent: # new in 2.21 + Enabled: false +Rails/EnumSyntax: # new in 2.26 + Enabled: false +Rails/WhereRange: # new in 2.25 + Enabled: false 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-github-rails.rb b/lib/rubocop-github-rails.rb index 02aa6adc..887b1df5 100644 --- a/lib/rubocop-github-rails.rb +++ b/lib/rubocop-github-rails.rb @@ -6,12 +6,10 @@ RuboCop::GitHub::Inject.rails_defaults! -require "rubocop/cop/github/rails_application_record" require "rubocop/cop/github/rails_controller_render_action_symbol" require "rubocop/cop/github/rails_controller_render_literal" require "rubocop/cop/github/rails_controller_render_paths_exist" require "rubocop/cop/github/rails_controller_render_shorthand" -require "rubocop/cop/github/rails_render_inline" require "rubocop/cop/github/rails_render_object_collection" require "rubocop/cop/github/rails_view_render_literal" require "rubocop/cop/github/rails_view_render_paths_exist" diff --git a/lib/rubocop-github.rb b/lib/rubocop-github.rb index 17c07e77..65bc8dc8 100644 --- a/lib/rubocop-github.rb +++ b/lib/rubocop-github.rb @@ -6,4 +6,5 @@ RuboCop::GitHub::Inject.default_defaults! +require "rubocop/cop/github/avoid_object_send_with_dynamic_method" require "rubocop/cop/github/insecure_hash_algorithm" diff --git a/lib/rubocop/cop/github/avoid_object_send_with_dynamic_method.rb b/lib/rubocop/cop/github/avoid_object_send_with_dynamic_method.rb new file mode 100644 index 00000000..e5e7677d --- /dev/null +++ b/lib/rubocop/cop/github/avoid_object_send_with_dynamic_method.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Cop + module GitHub + # Public: A Rubocop to discourage using methods like Object#send that allow you to dynamically call other + # methods on a Ruby object, when the method being called is itself completely dynamic. Instead, explicitly call + # methods by name. + # + # Examples: + # + # # bad + # foo.send(some_variable) + # + # # good + # case some_variable + # when "bar" + # foo.bar + # else + # foo.baz + # end + # + # # fine + # foo.send(:bar) + # foo.public_send("some_method") + # foo.__send__("some_#{variable}_method") + class AvoidObjectSendWithDynamicMethod < Base + MESSAGE_TEMPLATE = "Avoid using Object#%s with a dynamic method name." + SEND_METHODS = %i(send public_send __send__).freeze + CONSTANT_TYPES = %i(sym str const).freeze + + def on_send(node) + return unless send_method?(node) + return if method_being_sent_is_constrained?(node) + add_offense(source_range_for_method_call(node), message: MESSAGE_TEMPLATE % node.method_name) + end + + private + + def send_method?(node) + SEND_METHODS.include?(node.method_name) + end + + def method_being_sent_is_constrained?(node) + method_name_being_sent_is_constant?(node) || method_name_being_sent_is_dynamic_string_with_constants?(node) + end + + def method_name_being_sent_is_constant?(node) + method_being_sent = node.arguments.first + # e.g., `worker.send(:perform)` or `base.send("extend", Foo)` + CONSTANT_TYPES.include?(method_being_sent.type) + end + + def method_name_being_sent_is_dynamic_string_with_constants?(node) + method_being_sent = node.arguments.first + return false unless method_being_sent.type == :dstr + + # e.g., `foo.send("can_#{action}?")` + method_being_sent.child_nodes.any? { |child_node| CONSTANT_TYPES.include?(child_node.type) } + end + + def source_range_for_method_call(node) + begin_pos = + if node.receiver # e.g., for `foo.send(:bar)`, `foo` is the receiver + node.receiver.source_range.end_pos + else # e.g., `send(:bar)` + node.source_range.begin_pos + end + end_pos = node.loc.selector.end_pos + Parser::Source::Range.new(processed_source.buffer, begin_pos, end_pos) + end + end + end + end +end 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_controller_render_literal.rb b/lib/rubocop/cop/github/rails_controller_render_literal.rb index 0f2efd39..4ed294e1 100644 --- a/lib/rubocop/cop/github/rails_controller_render_literal.rb +++ b/lib/rubocop/cop/github/rails_controller_render_literal.rb @@ -9,7 +9,7 @@ module GitHub class RailsControllerRenderLiteral < Base include RenderLiteralHelpers - MSG = "render must be used with a string literal or an instance of a Class" + MSG = "render must be used with a string literal or an instance of a Class, and use literals for locals keys" def_node_matcher :ignore_key?, <<-PATTERN (pair (sym { 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/lib/rubocop/cop/github/rails_view_render_literal.rb b/lib/rubocop/cop/github/rails_view_render_literal.rb index c8563868..357b2ee1 100644 --- a/lib/rubocop/cop/github/rails_view_render_literal.rb +++ b/lib/rubocop/cop/github/rails_view_render_literal.rb @@ -54,7 +54,7 @@ def on_send(node) if render_literal?(node) && node.arguments.count > 1 locals = node.arguments[1] - elsif options_pairs = render_with_options?(node) + elsif option_pairs = render_with_options?(node) locals = option_pairs.map { |pair| locals_key?(pair) }.compact.first end diff --git a/lib/rubocop/cop/github/rails_view_render_shorthand.rb b/lib/rubocop/cop/github/rails_view_render_shorthand.rb index e465d8d3..05952425 100644 --- a/lib/rubocop/cop/github/rails_view_render_shorthand.rb +++ b/lib/rubocop/cop/github/rails_view_render_shorthand.rb @@ -6,6 +6,8 @@ module RuboCop module Cop module GitHub class RailsViewRenderShorthand < Base + extend AutoCorrector + MSG = "Prefer `render` partial shorthand" def_node_matcher :render_with_options?, <<-PATTERN @@ -26,9 +28,13 @@ def on_send(node) locals_key = option_pairs.map { |pair| locals_key?(pair) }.compact.first if option_pairs.length == 1 && partial_key - add_offense(node, message: "Use `render #{partial_key.source}` instead") + add_offense(node, message: "Use `render #{partial_key.source}` instead") do |corrector| + corrector.replace(node.source_range, "render #{partial_key.source}") + end elsif option_pairs.length == 2 && partial_key && locals_key - add_offense(node, message: "Use `render #{partial_key.source}, #{locals_key.source}` instead") + add_offense(node, message: "Use `render #{partial_key.source}, #{locals_key.source}` instead") do |corrector| + corrector.replace(node.source_range, "render #{partial_key.source}, #{locals_key.source}") + end end end end diff --git a/lib/rubocop/cop/github/render_literal_helpers.rb b/lib/rubocop/cop/github/render_literal_helpers.rb index 67417a54..29820b0e 100644 --- a/lib/rubocop/cop/github/render_literal_helpers.rb +++ b/lib/rubocop/cop/github/render_literal_helpers.rb @@ -41,7 +41,8 @@ module RenderLiteralHelpers PATTERN def hash_with_literal_keys?(hash) - hash.pairs.all? { |pair| literal?(pair.key) } + hash.children.all? { |child| child.pair_type? } && + hash.pairs.all? { |pair| literal?(pair.key) } end def render_view_component?(node) diff --git a/lib/version.rb b/lib/version.rb new file mode 100644 index 00000000..a9f2b951 --- /dev/null +++ b/lib/version.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +VERSION = "0.25.0" diff --git a/rubocop-github.gemspec b/rubocop-github.gemspec index 7b243dd4..43cb7e3d 100644 --- a/rubocop-github.gemspec +++ b/rubocop-github.gemspec @@ -1,25 +1,33 @@ # frozen_string_literal: true +require_relative "lib/version" + Gem::Specification.new do |s| s.name = "rubocop-github" - s.version = "0.19.0" + s.version = VERSION s.summary = "RuboCop GitHub" - s.description = "Code style checking for GitHub Ruby repositories " + s.description = "Code style checking for GitHub Ruby repositories" s.homepage = "https://github.com/github/rubocop-github" s.license = "MIT" + s.metadata = { + "source_code_uri" => "https://github.com/github/rubocop-github", + "documentation_uri" => "https://github.com/github/rubocop-github", + "bug_tracker_uri" => "https://github.com/github/rubocop-github/issues" + } + s.files = Dir["README.md", "STYLEGUIDE.md", "LICENSE", "config/*.yml", "lib/**/*.rb", "guides/*.md"] - s.add_dependency "rubocop", ">= 1.0.0" - s.add_dependency "rubocop-performance" - s.add_dependency "rubocop-rails" + s.required_ruby_version = ">= 3.1.0" - s.add_development_dependency "actionview" + s.add_dependency "rubocop", ">= 1.72" + s.add_dependency "rubocop-performance", ">= 1.24" + s.add_dependency "rubocop-rails", ">= 2.23" + + s.add_development_dependency "actionview", "~> 7.2.2.1" s.add_development_dependency "minitest" s.add_development_dependency "rake" - s.required_ruby_version = ">= 2.5.0" - s.email = "engineering@github.com" s.authors = "GitHub" end diff --git a/script/lint b/script/lint new file mode 100755 index 00000000..4c06652f --- /dev/null +++ b/script/lint @@ -0,0 +1,5 @@ +#! /usr/bin/env bash + +set -e + +bundle exec rake rubocop diff --git a/script/test b/script/test new file mode 100755 index 00000000..05166157 --- /dev/null +++ b/script/test @@ -0,0 +1,5 @@ +#! /usr/bin/env bash + +set -e + +bundle exec rake test diff --git a/test/cop_test.rb b/test/cop_test.rb index 4af051c1..cb4047ab 100644 --- a/test/cop_test.rb +++ b/test/cop_test.rb @@ -3,7 +3,7 @@ require "action_view" require "minitest" -class CopTest < MiniTest::Test +class CopTest < Minitest::Test def cop_class raise NotImplementedError end @@ -17,7 +17,7 @@ def setup def investigate(cop, src, filename = nil) processed_source = RuboCop::ProcessedSource.new(src, RUBY_VERSION.to_f, filename) - team = RuboCop::Cop::Team.new([cop], nil, raise_error: true) + team = RuboCop::Cop::Team.new([cop], cop.config, raise_error: true) report = team.investigate(processed_source) report.offenses end diff --git a/test/test_avoid_object_send_with_dynamic_method.rb b/test/test_avoid_object_send_with_dynamic_method.rb new file mode 100644 index 00000000..b176cbcc --- /dev/null +++ b/test/test_avoid_object_send_with_dynamic_method.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require_relative "cop_test" +require "minitest/autorun" +require "rubocop/cop/github/avoid_object_send_with_dynamic_method" + +class TestAvoidObjectSendWithDynamicMethod < CopTest + def cop_class + RuboCop::Cop::GitHub::AvoidObjectSendWithDynamicMethod + end + + def test_offended_by_send_call + offenses = investigate cop, <<-RUBY + def my_method(foo) + foo.send(@some_ivar) + end + RUBY + assert_equal 1, offenses.size + assert_equal "#{cop.name}: Avoid using Object#send with a dynamic method name.", offenses.first.message + end + + def test_offended_by_public_send_call + offenses = investigate cop, <<-RUBY + foo.public_send(bar) + RUBY + assert_equal 1, offenses.size + assert_equal "#{cop.name}: Avoid using Object#public_send with a dynamic method name.", offenses.first.message + end + + def test_offended_by_call_to___send__ + offenses = investigate cop, <<-RUBY + foo.__send__(bar) + RUBY + assert_equal 1, offenses.size + assert_equal "#{cop.name}: Avoid using Object#__send__ with a dynamic method name.", offenses.first.message + end + + def test_offended_by_send_calls_without_receiver + offenses = investigate cop, <<-RUBY + send(some_method) + public_send(@some_ivar) + __send__(a_variable, "foo", "bar") + RUBY + assert_equal 3, offenses.size + assert_equal "#{cop.name}: Avoid using Object#send with a dynamic method name.", offenses[0].message + assert_equal "#{cop.name}: Avoid using Object#public_send with a dynamic method name.", offenses[1].message + assert_equal "#{cop.name}: Avoid using Object#__send__ with a dynamic method name.", offenses[2].message + end + + def test_unoffended_by_other_method_calls + offenses = investigate cop, <<-RUBY + foo.bar(arg1, arg2) + case @some_ivar + when :foo + baz.foo + when :bar + baz.bar + end + puts "public_send" if send? + RUBY + assert_equal 0, offenses.size + end + + def test_unoffended_by_send_calls_to_dynamic_methods_that_include_hardcoded_strings + offenses = investigate cop, <<-'RUBY' + foo.send("can_#{action}?") + foo.public_send("make_#{SOME_CONSTANT}") + RUBY + assert_equal 0, offenses.size + end + + def test_unoffended_by_send_calls_without_dynamic_methods + offenses = investigate cop, <<-RUBY + base.send :extend, ClassMethods + foo.public_send(:bar) + foo.__send__("bar", arg1, arg2) + RUBY + assert_equal 0, offenses.size + end +end diff --git a/test/test_insecure_hash_algorithm.rb b/test/test_insecure_hash_algorithm.rb index 597450b8..317562e1 100644 --- a/test/test_insecure_hash_algorithm.rb +++ b/test/test_insecure_hash_algorithm.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/insecure_hash_algorithm" @@ -10,7 +10,7 @@ def cop_class end def make_cop(allowed:) - config = RuboCop::Config.new({"GitHub/InsecureHashAlgorithm" => {"Allowed" => allowed}}) + config = RuboCop::Config.new({ "GitHub/InsecureHashAlgorithm" => { "Allowed" => allowed } }) cop_class.new(config) end @@ -57,7 +57,7 @@ def secret_hmac RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_digest_method_md5_str @@ -70,7 +70,7 @@ def h RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_digest_method_md5_symbol @@ -83,7 +83,7 @@ def h RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_digest_method_sha256_str @@ -122,7 +122,7 @@ class Something RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_alias_for_openssl_digest_md5 @@ -133,7 +133,7 @@ class Something RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_alias_for_digest_sha1 @@ -144,7 +144,7 @@ class Something RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_alias_for_openssl_digest_sha1 @@ -155,7 +155,7 @@ class Something RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_alias_for_digest_sha256 @@ -200,7 +200,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_md5_hexdigest @@ -213,7 +213,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_md5_digest_by_name @@ -226,7 +226,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha1_digest_by_name @@ -239,7 +239,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha1_hexdigest_by_name_mixed_case @@ -252,7 +252,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha256_digest_by_name @@ -277,7 +277,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha1_hmac_by_name @@ -290,7 +290,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha256_hmac_by_name @@ -315,7 +315,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha1_digest_instance_by_name @@ -328,7 +328,7 @@ def something(str) RUBY assert_equal 1, offenses.count - assert_equal cop_class::MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::MSG}", offenses.first.message end def test_openssl_sha256_digest_instance_by_name @@ -367,7 +367,7 @@ def uuid RUBY assert_equal 1, offenses.count - assert_equal cop_class::UUID_V3_MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::UUID_V3_MSG}", offenses.first.message end def test_uuid_v3_with_md5_allowed @@ -405,7 +405,7 @@ def uuid RUBY assert_equal 1, offenses.count - assert_equal cop_class::UUID_V5_MSG, offenses.first.message + assert_equal "#{cop.name}: #{cop_class::UUID_V5_MSG}", offenses.first.message end def test_uuid_v5_with_sha1_allowed 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_controller_render_action_symbol.rb b/test/test_rails_controller_render_action_symbol.rb index 5731d82e..9b67a2c5 100644 --- a/test/test_rails_controller_render_action_symbol.rb +++ b/test/test_rails_controller_render_action_symbol.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/rails_controller_render_action_symbol" @@ -47,7 +47,7 @@ def update RUBY assert_equal 3, offenses.count - expected_message = "Prefer `render` with string instead of symbol" + expected_message = "#{cop.name}: Prefer `render` with string instead of symbol" assert_equal expected_message, offenses[0].message assert_equal expected_message, offenses[1].message assert_equal expected_message, offenses[2].message diff --git a/test/test_rails_controller_render_literal.rb b/test/test_rails_controller_render_literal.rb index 9da2f16c..eb13928f 100644 --- a/test/test_rails_controller_render_literal.rb +++ b/test/test_rails_controller_render_literal.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/rails_controller_render_literal" @@ -274,7 +274,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_implicit_with_layout_offense @@ -287,7 +289,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_implicit_with_status_offense @@ -300,7 +304,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_variable_offense @@ -313,7 +319,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_to_string_variable_offense @@ -326,7 +334,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_action_variable_offense @@ -339,7 +349,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_template_variable_offense @@ -352,7 +364,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_partial_variable_offense @@ -365,7 +379,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_template_with_layout_variable_offense @@ -378,7 +394,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_template_variable_with_layout_offense @@ -391,7 +409,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_shorthand_static_locals_no_offsense @@ -442,6 +462,17 @@ def index assert_equal 1, offenses.count end + def test_render_literal_splat_locals_offense + offenses = investigate cop, <<-RUBY, "app/controllers/products_controller.rb" + class ProductsController < ActionController::Base + def index + render "products/product", locals: { **locals } + end + end + RUBY + + assert_equal 1, offenses.count + end def test_render_literal_dynamic_local_key_offense offenses = investigate cop, <<-RUBY, "app/controllers/products_controller.rb" diff --git a/test/test_rails_controller_render_shorthand.rb b/test/test_rails_controller_render_shorthand.rb index 0c16bd56..f4bb111a 100644 --- a/test/test_rails_controller_render_shorthand.rb +++ b/test/test_rails_controller_render_shorthand.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/rails_controller_render_shorthand" @@ -55,9 +55,9 @@ def confirm RUBY assert_equal 3, offenses.count - assert_equal "Use `render \"edit\"` instead", offenses[0].message - assert_equal "Use `render \"new\"` instead", offenses[1].message - assert_equal "Use `render \"confirm.html.erb\"` instead", offenses[2].message + assert_equal "#{cop.name}: Use `render \"edit\"` instead", offenses[0].message + assert_equal "#{cop.name}: Use `render \"new\"` instead", offenses[1].message + assert_equal "#{cop.name}: Use `render \"confirm.html.erb\"` instead", offenses[2].message end def test_render_template_offense @@ -78,8 +78,8 @@ def edit RUBY assert_equal 3, offenses.count - assert_equal "Use `render \"books/new\"` instead", offenses[0].message - assert_equal "Use `render \"books/show\", locals: { book: @book }` instead", offenses[1].message - assert_equal "Use `render \"books/edit.html.erb\", status: :ok, layout: \"application\"` instead", offenses[2].message + assert_equal "#{cop.name}: Use `render \"books/new\"` instead", offenses[0].message + assert_equal "#{cop.name}: Use `render \"books/show\", locals: { book: @book }` instead", offenses[1].message + assert_equal "#{cop.name}: Use `render \"books/edit.html.erb\", status: :ok, layout: \"application\"` instead", offenses[2].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 diff --git a/test/test_rails_render_object_collection.rb b/test/test_rails_render_object_collection.rb index 7786dedd..c13ec9af 100644 --- a/test/test_rails_render_object_collection.rb +++ b/test/test_rails_render_object_collection.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/rails_render_object_collection" @@ -27,7 +27,7 @@ def test_render_partial_with_object_offense ERB assert_equal 1, offenses.count - assert_equal "Avoid `render object:`, instead `render partial: \"account\", locals: { account: @buyer }`", offenses[0].message + assert_equal "#{cop.name}: Avoid `render object:`, instead `render partial: \"account\", locals: { account: @buyer }`", offenses[0].message end def test_render_collection_with_object_offense @@ -36,7 +36,7 @@ def test_render_collection_with_object_offense ERB assert_equal 1, offenses.count - assert_equal "Avoid `render collection:`", offenses[0].message + assert_equal "#{cop.name}: Avoid `render collection:`", offenses[0].message end def test_render_spacer_template_with_object_offense @@ -45,6 +45,6 @@ def test_render_spacer_template_with_object_offense ERB assert_equal 1, offenses.count - assert_equal "Avoid `render collection:`", offenses[0].message + assert_equal "#{cop.name}: Avoid `render collection:`", offenses[0].message end end diff --git a/test/test_rails_view_render_literal.rb b/test/test_rails_view_render_literal.rb index b983d5f8..7c081dd3 100644 --- a/test/test_rails_view_render_literal.rb +++ b/test/test_rails_view_render_literal.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/rails_view_render_literal" @@ -43,7 +43,7 @@ def test_render_variable_offense ERB assert_equal 2, offenses.count - assert_equal "render must be used with a literal template and use literals for locals keys", offenses[0].message + assert_equal "#{cop.name}: render must be used with a literal template and use literals for locals keys", offenses[0].message end def test_render_component_no_offense @@ -102,7 +102,7 @@ def test_render_layout_variable_literal_no_offense ERB assert_equal 1, offenses.count - assert_equal "render must be used with a literal template and use literals for locals keys", offenses[0].message + assert_equal "#{cop.name}: render must be used with a literal template and use literals for locals keys", offenses[0].message end def test_render_inline_no_offense @@ -145,6 +145,14 @@ def test_render_literal_dynamic_local_key_offense assert_equal 1, offenses.count end + def test_render_literal_splat_locals_offense + offenses = erb_investigate cop, <<-ERB, "app/views/products/index.html.erb" + <%= render "products/product", { **locals } %> + ERB + + assert_equal 1, offenses.count + end + def test_render_options_static_locals_no_offense offenses = erb_investigate cop, <<-ERB, "app/views/products/index.html.erb" <%= render partial: "products/product", locals: { product: product } %> @@ -168,4 +176,12 @@ def test_render_options_dynamic_local_key_offense assert_equal 1, offenses.count end + + def test_render_options_local_splat_offense + offenses = erb_investigate cop, <<-ERB, "app/views/products/index.html.erb" + <%= render partial: "products/product", locals: { **locals } %> + ERB + + assert_equal 1, offenses.count + end end diff --git a/test/test_rails_view_render_shorthand.rb b/test/test_rails_view_render_shorthand.rb index 22ff453a..0077ceea 100644 --- a/test/test_rails_view_render_shorthand.rb +++ b/test/test_rails_view_render_shorthand.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "./cop_test" +require_relative "cop_test" require "minitest/autorun" require "rubocop/cop/github/rails_view_render_shorthand" @@ -31,7 +31,7 @@ def test_render_partial_offense ERB assert_equal 1, offenses.count - assert_equal "Use `render \"foo/bar\"` instead", offenses[0].message + assert_equal "#{cop.name}: Use `render \"foo/bar\"` instead", offenses[0].message end def test_render_partial_with_locals_offense @@ -40,7 +40,7 @@ def test_render_partial_with_locals_offense ERB assert_equal 1, offenses.count - assert_equal "Use `render \"foo/bar\", { bar: 42 }` instead", offenses[0].message + assert_equal "#{cop.name}: Use `render \"foo/bar\", { bar: 42 }` instead", offenses[0].message end def test_render_layout_no_offense