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

Unsafe autocorrect with Rails/Pluck #826

Closed
axlekb opened this issue Oct 22, 2022 · 3 comments · Fixed by #924
Closed

Unsafe autocorrect with Rails/Pluck #826

axlekb opened this issue Oct 22, 2022 · 3 comments · Fixed by #924
Assignees
Labels
enhancement New feature or request

Comments

@axlekb
Copy link

axlekb commented Oct 22, 2022

If column name is aliased in a select clause, using .pluck('alias_column') in place of .map { |post| post['alias_column'] } will cause an ActiveRecord::StatementInvalid

Cop docs:

# bad
Post.published.map { |post| post[:title] }
[{ a: :b, c: :d }].collect { |el| el[:a] }

# good
Post.published.pluck(:title)
[{ a: :b, c: :d }].pluck(:a)

Expected behavior

Autocorrect should not result in changes that raise an error

Actual behavior

Autocorrect causes ActiveRecord::StatementInvalid

Steps to reproduce the problem

  • Add a #join and #select clause to an ActiveRecord::Relation. (e.g. .select("table_1.*", "table_2.id AS table_2_id"))

Using .map { |x| x['table_2_id'] } returns an Array of the aliased column
Using .pluck('table_2_id') raises an ActiveRecord::StatementInvalid because table_2_id is not a table column
Using .to_a.pluck('table_2_id') returns an Array of the aliased column

RuboCop version

$ rubocop -V
1.37.0 (using Parser 3.1.2.1, rubocop-ast 1.23.0, running on ruby 3.0.4) [arm64-darwin21]
  - rubocop-performance 1.15.0
  - rubocop-rails 2.17.0
  - rubocop-rspec 2.13.2
@jcoyne
Copy link
Contributor

jcoyne commented Oct 26, 2022

See further examples in #842 where the target is not an AR::Relation, but a simple ruby Hash.

@vlad-pisanov
Copy link
Contributor

+1 unfortunately, the cop is inherently unsafe when column aliasing is used:

User.select('id AS id2').map { |user| user[:id2] } # works ✔️
User.select('id AS id2').pluck(:id2)               # ERROR:  column "id2" does not exist ❌
User.select('id AS id2').to_a.pluck(:id2)          # works ✔️

@Drenmi Drenmi added the enhancement New feature or request label Nov 9, 2022
@Drenmi
Copy link
Contributor

Drenmi commented Nov 9, 2022

I suspect the most we can do is mark this auto-correct as unsafe.

@Drenmi Drenmi self-assigned this Nov 9, 2022
@koic koic closed this as completed in #924 Jan 27, 2023
koic added a commit that referenced this issue Jan 31, 2023
#826 reports an error on alias column, not on `alias_attribute`.
In fact, `alias_attribute` doesn't seem to cause an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants