Skip to content

Conversation

@beengine
Copy link

Please ensure your pull request includes the following:

  • Description of changes
  • Changes have related RSpec tests that ensure functionality does not break

This PR relates to the issue #396 and partially covers it.

Summary of Changes

Configuration Updates (.rubocop.yml)

  • Added exclusions for Lint/ConstantDefinitionInBlock in spec files (test-specific pattern)
  • Added exclusions for Naming/PredicateMethod in lib and spec (adapter pattern compatibility)

Code Style Improvements

Hash & Alignment:

  • Fixed hash key alignments across multiple files
  • Improved indentation consistency

String & Argument Simplification:

  • Removed redundant flatten calls on already-flat arrays (bcrypt.rb, spec files)
  • Changed join('') to join (empty string is default)
  • Simplified proc { |r| r.email } to :email in validations

Conditional Logic:

  • Fixed nested conditionals (Style/SoleNestedConditional) in:
    • brute_force_protection.rb - Combined conditions for unlock check
    • user_activation.rb - Combined conditions for activation state check
  • Fixed if/unless modifier usage for single-line conditionals

Method Improvements:

  • Replaced unpack('m').first with unpack1('m') for better performance
  • Used anonymous block forwarding where appropriate
  • Improved symbol proc usage (&:method_name)

Provider URL Refactoring:

  • Refactored provider_url method in controller_oauth2_spec.rb for readability:
    • Extracted common config and redirect_uri variables
    • Split long URLs across multiple lines
    • Improved maintainability for OAuth provider URL construction

Removed Redundancies

  • Removed redundant constant base references (::)
  • Removed redundant cop disable directives
  • Removed redundant parentheses in conditionals
  • Fixed ambiguous operator precedence warnings

Linting Fixes

  • Fixed Lint/UnusedBlockArgument - Used underscore prefix for unused block args
  • Fixed Lint/SendWithMixinArgument - Corrected mixin inclusion patterns
  • Fixed Lint/AmbiguousOperatorPrecedence - Added clarity to complex expressions

Empty Line Management

  • Added/removed empty lines for better code organization
  • Improved visual separation of logical code blocks

Files with Significant Changes

Library Files:

  • lib/sorcery/crypto_providers/bcrypt.rb - Removed redundant flatten
  • lib/sorcery/crypto_providers/aes256.rb - Used unpack1 for efficiency
  • lib/sorcery/model/submodules/brute_force_protection.rb - Simplified nested conditionals
  • lib/sorcery/model/submodules/user_activation.rb - Simplified nested conditionals

Spec Files:

  • spec/controllers/controller_oauth2_spec.rb - Major refactor of provider_url method
  • spec/shared_examples/user_shared_examples.rb - Multiple style fixes
  • spec/sorcery_crypto_providers_spec.rb - Removed redundant flatten calls

.rubocop_todo.yml Cleanup

Removed violations for the following cops (all now fixed):

  • Style/SoleNestedConditional (4 offenses)
  • Style/UnpackFirst (1 offense)
  • Style/RedundantArrayFlatten (5 offenses)
  • Style/SymbolProc (1 offense)
  • Hash alignment offenses
  • Empty line offenses
  • And many more...

@beengine beengine changed the title Fix rubocop offences Fix rubocop offenses Oct 24, 2025
@willnet willnet requested review from Copilot and willnet October 28, 2025 05:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes RuboCop offenses across the codebase, focusing on code style improvements, redundancy removal, and conditional logic simplification while maintaining backward compatibility.

Key changes:

  • Removed redundant constant base references (::) throughout spec and lib files
  • Simplified nested conditionals by combining conditions where appropriate
  • Applied Ruby idioms (e.g., unpack1 instead of unpack.first, anonymous block forwarding)

Reviewed Changes

Copilot reviewed 46 out of 55 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/spec_helper.rb Removed redundant :: prefix from constant references
spec/sorcery_crypto_providers_spec.rb Removed redundant flatten calls and changed comment style to uppercase NOTE
spec/shared_examples/*.rb Removed :: prefixes, updated block argument naming, changed expect block delimiters
spec/rails_app/config/*.rb Converted multi-line conditionals to single-line modifiers
spec/providers/vk_spec.rb Fixed redundant regex escape in URL pattern
spec/controllers/controller_oauth2_spec.rb Refactored provider_url method for better readability, removed :: prefixes, updated string quotes
lib/sorcery/test_helpers/*.rb Applied anonymous block forwarding, updated hash syntax
lib/sorcery/providers/*.rb Simplified authorize_url calls, fixed operator precedence, updated regex usage
lib/sorcery/model/*.rb Simplified nested conditionals, applied anonymous block forwarding
lib/sorcery/crypto_providers/*.rb Removed redundant flatten calls, added empty lines after include
lib/sorcery/controller/*.rb Simplified conditionals, applied anonymous block forwarding, reorganized attr_accessor declarations
lib/sorcery/adapters/*.rb Updated hash syntax, applied anonymous block forwarding, simplified conditionals
lib/sorcery/engine.rb Replaced send(:include) with direct include call
lib/generators/sorcery/*.rb Fixed redundant parentheses, updated regex usage
.rubocop_todo.yml Removed fixed offense entries
.rubocop.yml Added exclusions for Naming/PredicateMethod and Lint/ConstantDefinitionInBlock

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@willnet willnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. It’s easier to review if each cop fix is a separate commit, so if possible, please create commits in smaller chunks.

The failing CI should pass once you rebase onto the current master.

- Add empty lines after module inclusion (Layout/EmptyLinesAfterModuleInclusion)
- Add empty lines around attribute accessors (Layout/EmptyLinesAroundAttributeAccessor)
- Remove extra empty lines (Layout/EmptyLines)
- Use inline RuboCop disable for dynamic submodule inclusion to avoid rubocop looping
- Align hash values consistently using separators
- Fix first hash element indentation (Layout/FirstHashElementIndentation)
- Standardize spacing in hash literals across configuration files, adapters, and providers
 The Lint/ConstantDefinitionInBlock cop was disabled for specs because RSpec tests legitimately need to define classes and constants inside test blocks for testing purposes
Lint/HandleExceptions was renamed to Lint/SuppressedException
`include` methods were private methods until Ruby 2.0, so for ruby 2.0+ `include` can be called by public method
In Ruby 3.1, anonymous block forwarding has been added.
The method update_attributes returns a boolean (updated_count == 1), so RuboCop thinks it should be named update_attributes?. However, this method is not semantically a predicate - it's performing an action (updating attributes) and returning a success/failure indicator. So check is disabled.
  - Group attr_accessor declarations (Style/AccessorGrouping)
  - Use modern hash syntax with colons (Style/HashSyntax)
  - Assign conditionals to variables (Style/ConditionalAssignment)
  - Use do/end blocks for multi-line expect (Style/BlockDelimiters)
  - Capitalize comment annotations (Style/CommentAnnotation)
Remove redundant :: from constant references (Style/RedundantConstantBase)
- Remove unnecessary parentheses in format() and conditional expressions
- Replace regex arguments with string arguments in gsub() calls
- Remove unnecessary regex escape sequences in URL patterns
- Convert multi-line if/unless statements to single-line modifier syntax where appropriate
- Split long string literals in spec helper methods for better readability
- Standardize string quotes in OAuth specs (double to single quotes)
  - Use string interpolation instead of concatenation (Style/StringConcatenation)
  - Use symbol proc shorthand where applicable (Style/SymbolProc)
  - Use unpack1 instead of unpack.first (Style/UnpackFirst)
@beengine beengine force-pushed the fix-rubocop-offences branch from 7a5dcb5 to 943bf87 Compare October 28, 2025 13:16
@beengine beengine requested a review from willnet October 28, 2025 13:26
The RSpec-recommended way to temporarily define constants with stub_const
@beengine beengine force-pushed the fix-rubocop-offences branch from 33edab1 to be5285c Compare October 30, 2025 09:14
@beengine beengine requested a review from willnet October 30, 2025 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants