Enforce new RuboCop cops for all Ruby files #566
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Only safe autocorrections used. Command: bundle exec rubocop --autocorrect --disable-uncorrectable version: 1.57.2 (using Parser 3.2.2.4, rubocop-ast 1.30.0, running on ruby 3.1.4) [x86_64-linux] See the docs: https://docs.rubocop.org/rubocop/usage/auto_correct.html 704 files inspected, 8138 offenses detected, 7399 offenses corrected, 720 more offenses can be corrected with `rubocop -A`. Note there occurred one error for the file "db/seeds.rb", which will be fixed manually in subsequent commits.
This reverts commit b067b2f.
704 files inspected, 3912 offenses detected, 3912 offenses corrected
704 files inspected, 3171 offenses detected, 2432 offenses corrected, 720 more offenses can be corrected with `rubocop -A` Command: bundle exec rubocop --autocorrect --disable-uncorrectable
This is since we only use the instance variable to provide a *default* value for some params, so the helper methods can still be reused.
The respective comments were added manually, not automatically.
Replaced `update_all` by `update` and `Time.now` by `Time.current`
This is to avoid Rails/UnknownEnv error
Command used: bundle exec rubocop --autocorrect-all --disable-uncorrectable 704 files inspected, 911 offenses detected, 909 offenses corrected 2 errors occurred: An error occurred while Style/StringConcatenation cop was inspecting mampf/app/models/medium.rb:871:30. An error occurred while Layout/LineLength cop was inspecting mampf/db/seeds.rb. I manually fixed the error in medium.rb. I checked the seeds.rb and dit not find any error there.
Command used: bundle exec rubocop --autocorrect --disable-uncorrectable 704 files inspected, 371 offenses detected, 350 offenses corrected, 13 more offenses can be corrected with `rubocop -A`
fosterfarrell9
requested changes
Dec 9, 2023
See this comment: #566 (comment)
This is because we think it's cleaner not to have to write "dependent: nil". Also removed the respective rubocop:todo comments
Splines
commented
Dec 14, 2023
fosterfarrell9
approved these changes
Dec 18, 2023
Splines
added a commit
that referenced
this pull request
Jan 6, 2024
* Let RuboCop autocorrect all Ruby files (safe) Only safe autocorrections used. Command: bundle exec rubocop --autocorrect --disable-uncorrectable version: 1.57.2 (using Parser 3.2.2.4, rubocop-ast 1.30.0, running on ruby 3.1.4) [x86_64-linux] See the docs: https://docs.rubocop.org/rubocop/usage/auto_correct.html 704 files inspected, 8138 offenses detected, 7399 offenses corrected, 720 more offenses can be corrected with `rubocop -A`. Note there occurred one error for the file "db/seeds.rb", which will be fixed manually in subsequent commits. * Revert "Let RuboCop autocorrect all Ruby files (safe)" This reverts commit b067b2f. * Force use of explicit hash literal value (HashSyntax) * Only correct string literals to double quotes (safe) 704 files inspected, 3912 offenses detected, 3912 offenses corrected * Allow use of method ".touch" * Autocorrect all other specified cops (safe) 704 files inspected, 3171 offenses detected, 2432 offenses corrected, 720 more offenses can be corrected with `rubocop -A` Command: bundle exec rubocop --autocorrect --disable-uncorrectable * Fix Layout/IndentationConsistency (manual) * Fix (or disable) Layout/LineLength * Fix all Style/ cops * Fix Lint/ cops (3 still left open) * Rename `get_votes_count` to `votes_count` * Fix other Naming/ cops (1 still left open) * Disable `Rails/HelperInstanceVariable` in `quizzes_helper.rb` This is since we only use the instance variable to provide a *default* value for some params, so the helper methods can still be reused. * Fix all Rails/InverseOf cops * Disable Rails/ cops in existing db migrations The respective comments were added manually, not automatically. * Fix Rails/SkipsModelValidations (1 left open) Replaced `update_all` by `update` and `Time.now` by `Time.current` * Disable Rails/OutputSafety for one line * Fix Rails/I18n related cops * Add custom env variable to rubocop This is to avoid Rails/UnknownEnv error * Disable Performance/CollectionLiteralInLoop in some tests * Merge two duplicates groups in Gemfile together * Automatically autocorrect cops (unsafe) Command used: bundle exec rubocop --autocorrect-all --disable-uncorrectable 704 files inspected, 911 offenses detected, 909 offenses corrected 2 errors occurred: An error occurred while Style/StringConcatenation cop was inspecting mampf/app/models/medium.rb:871:30. An error occurred while Layout/LineLength cop was inspecting mampf/db/seeds.rb. I manually fixed the error in medium.rb. I checked the seeds.rb and dit not find any error there. * Fix missing `Time.zone.now` * Add Style/MethodCallWithArgsParentheses and autofix (safe) Command used: bundle exec rubocop --autocorrect --disable-uncorrectable 704 files inspected, 371 offenses detected, 350 offenses corrected, 13 more offenses can be corrected with `rubocop -A` * Fix Layout/ cops * Fix or disable Style/ cops * Delete weird random character * Fix Security/ cops * Fix Performance/ cops * Fix wrong namespace for a cop * Manually ignore more db/ cop violations * Delete empty test files * Fix line to long * Temporarily disable Rails/LexicallyScopedActionFilter * Fix <= logical bug (registration threshold) * remove duplicated restricted? method * change private methods to non-private methods * remove duplicate method * fix typo * change inverse_of relation * fix existing inverse_of associations * rename duplicate announcements method * undo premature change * add namespace to constant * remove unnecessary logging * remove non existing action from before filter * Rewrite new registrations query with timeframe * Allow touch_all method and replace problematic update calls See this comment: #566 (comment) * Disable Rails/HasManyOrHasOneDependent rule This is because we think it's cleaner not to have to write "dependent: nil". Also removed the respective rubocop:todo comments * Fix unwanted "," character * Get rid of weird "foo" check * remove unused restricted? methods * Replace problematic touch_all by touch * Fix wrong manual Style/ correction of 5081aaa * Fix Rails/OutputSafety * Remove duplicate dates before save see #566 (comment) --------- Co-authored-by: fosterfarrell9 <[email protected]>
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rubocop version information:
Commands used:
Notable manual changes
Time.currentinstead ofTime.now. Refupdateinstead ofupdate_all. RefNotable manual changes (explicit) / Questions from my side
Current status (2023-12-03 15:30):
rubocop:todo Rails/cops leftrubocop:todo Naming/cop left -> easily fixablerubocop:todo Lint/cops leftrubocop:todo Metrics/cops left -> probably won't fix in this PR (will stay as todo note)Some open points or commits to take especially care of (please mark as checked):
registrations_controller.rb->check_registration_limituser_cleaner.rbfor long regex statements.Devise.setup(see Basecamp)shrine.rb-> addedsubmission_pathquizzes_controller.rb->init_values(quiz_round_params[:save_probe])medium.rb´ (one still has to be fixedis_restricted?`)submissions.rb: are the methods at the bottom really supposed to be private class methods?db.rake->ensure_formatStandardErrorinregistrations_controlleras well as inlog_fail? Please provide a comment therehas_documents?method inassignments_helperif we have the same method inassignment.rb(over there: renamed todocuments?user.rb-> The methodhas_tutorialswas never used (I've made a search for that word), so I deleted itRails/HasManyOrHasOneDependenttodos -> won't fix on my own.Rails/InverseOfcops but am not 100% sure that this is the way it should work. I followed the examples from here but have a feeling summarized well by this comment.12Rails/UniqueValidationWithoutIndex-> won't fix on my own and probably not in this PR as it may require changing db columns, i.e. add new migrations, right?course.rb->question_ids_for_quiz, there is an unused variable. Should we return it?submission.rb->filename_for_bulk_downloadNotable unsafe corrections (incomplete list!)
Note that autocorrects running on old migrations were not committed.
video_uploader.rb->ppchanged toRails.logger.debugBefore merging
rubocop:enablethat don't have matchingrubocop:todoorrubocop:enableblock beforehand (rubocop can search for that, it is a cop on its own)bundle exec rubocop -Ashould not show any warning/error any more (see also CI/CD pipeline)Todo
MethodCallWithArgsParentheses