Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive offense with Rails/UniqueValidationWithoutIndex using expression indexes #215

Closed
ZilvinasKucinskas opened this issue Mar 25, 2020 · 0 comments · Fixed by #217
Labels
bug Something isn't working

Comments

@ZilvinasKucinskas
Copy link

Rails 5 added support for expression indexes as follows:

def change
  add_index :emails,
            'lower(address)',
            name: "index_emails_on_address_unique",
            unique: true
end

However, it does not seem to be recognised by Rails/UniqueValidationWithoutIndex rule. I have added a test case below that fails, but should pass instead.

Expected behavior

No offense triggered.

Actual behavior

Offense is being triggered with Uniqueness validation should be with a unique index message.

Steps to reproduce the problem

Here is the test case we can put into spec/rubocop/cop/rails/unique_validation_without_index_spec.rb:

    context 'with expression indexes' do
      let(:schema) { <<~RUBY }
        ActiveRecord::Schema.define(version: 2020_02_02_075409) do
          create_table "emails", force: :cascade do |t|
            t.string "address", null: false
            t.index "lower(address)", name: "index_emails_on_LOWER_address", unique: true
          end
        end
      RUBY

      it 'registers no offense' do
        expect_no_offenses(<<~RUBY)
          class Email
            validates :address, presence: true, uniqueness: { case_sensitive: false }, email: true
          end
        RUBY
      end
    end

RuboCop version

$ [bundle exec] rubocop -V
0.80.1 (using Parser 2.7.0.5, running on ruby 2.6.5 x86_64-darwin18)
@koic koic added the bug Something isn't working label Mar 26, 2020
koic added a commit to koic/rubocop-rails that referenced this issue Mar 26, 2020
…houtIndex`

Fixes rubocop#215.

This PR fixes a false positive for `Rails/UniqueValidationWithoutIndex`
when using Expression Indexes for PostgreSQL.

Since it is difficult to predict a kind of string described in Expression Indexes,
I decided to determine whether an Expression Indexes included a column name.
There is a possibility of false negative, but in most cases I expect it works.

## References

- https://www.postgresql.org/docs/12/indexes-expressional.html
- rails/rails@edc2b77
koic added a commit to koic/rubocop-rails that referenced this issue Mar 26, 2020
…houtIndex`

Fixes rubocop#215.

This PR fixes a false positive for `Rails/UniqueValidationWithoutIndex`
when using Expression Indexes for PostgreSQL.

Since it is difficult to predict a kind of string described in Expression Indexes,
this implementation judges based on whether or not an Expression Indexes include
a column name.
There is a possibility of false negative, but in most cases I expect it works.

## References

- https://www.postgresql.org/docs/12/indexes-expressional.html
- rails/rails@edc2b77
koic added a commit to koic/rubocop-rails that referenced this issue Mar 26, 2020
…houtIndex`

Fixes rubocop#215.

This PR fixes a false positive for `Rails/UniqueValidationWithoutIndex`
when using Expression Indexes for PostgreSQL.

Since it is difficult to predict a kind of string described in Expression Indexes,
this implementation judges based on whether or not an Expression Indexes include
a column name.
There is a possibility of false negative, but in most cases I expect it works.

## References

- https://www.postgresql.org/docs/12/indexes-expressional.html
- rails/rails@edc2b77
koic added a commit to koic/rubocop-rails that referenced this issue Mar 26, 2020
…houtIndex`

Fixes rubocop#215.

This PR fixes a false positive for `Rails/UniqueValidationWithoutIndex`
when using Expression Indexes for PostgreSQL.

Since it is difficult to predict a kind of string described in Expression Indexes,
this implementation judges based on whether or not an Expression Indexes include
a column name.
There is a possibility of false negative, but in most cases I expect it works.

## References

- https://www.postgresql.org/docs/12/indexes-expressional.html
- rails/rails@edc2b77
@koic koic closed this as completed in #217 Mar 27, 2020
koic added a commit that referenced this issue Mar 27, 2020
…idation_without_index

[Fix #215] Fix a false positive for `Rails/UniqueValidationWithoutIndex`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants