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

[Fix #9520] Fix an incorrect auto-correct for Style/MultipleComparison #9521

Conversation

koic
Copy link
Member

@koic koic commented Feb 16, 2021

Fixes #9520.

This PR fixes an incorrect auto-correct for Style/MultipleComparison when comparing a variable with multiple items in if and elsif conditions.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@koic koic force-pushed the fix_incorrect_autocorrect_for_style_multiple_comparison branch from 9de7788 to 82f77d7 Compare February 16, 2021 16:40
@adfoster-r7
Copy link

adfoster-r7 commented Feb 16, 2021

Thanks for looking into this! I've just verified that the code snippet from the issue is fixed

However it looks like there's still an issue else where in the file - invalid code is generated for the middle if statement:

diff --git a/lib/rex/post/meterpreter/extensions/stdapi/sys/registry.rb b/lib/rex/post/meterpreter/extensions/stdapi/sys/registry.rb
index 32a2fcd89a..270cc02c9e 100644
--- a/lib/rex/post/meterpreter/extensions/stdapi/sys/registry.rb
+++ b/lib/rex/post/meterpreter/extensions/stdapi/sys/registry.rb
@@ -215,7 +215,7 @@ class Registry
     request.add_tlv(TLV_TYPE_VALUE_NAME, name)
     request.add_tlv(TLV_TYPE_VALUE_TYPE, type)
 
-    if type == REG_SZ || type == REG_MULTI_SZ
+    if [REG_SZ, REG_MULTI_SZ].include?(type)
       data += "\x00"
     elsif (type == REG_DWORD)
       data = [ data.to_i ].pack("V")
@@ -237,7 +237,7 @@ class Registry
     request.add_tlv(TLV_TYPE_VALUE_NAME, name)
     request.add_tlv(TLV_TYPE_VALUE_TYPE, type)
 
-    if type == REG_SZ || type == REG_MULTI_SZ
+    if [REG_SZ, REG_MULTI_SZ, REG_SZ, REG_MULTI_SZ, REG_SZ, REG_MULTI_SZ].include?(type) <-- *Now different*
       data += "\x00"
     elsif type == REG_DWORD
       data = [data.to_i].pack('V')
@@ -373,13 +373,13 @@ class Registry
   # for converting HKLM as a string into its actual integer representation.
   #
   def self.key2str(key)
-    if (key == 'HKLM' or key == 'HKEY_LOCAL_MACHINE')
+    if (['HKLM', 'HKEY_LOCAL_MACHINE'].include?(key))
       return HKEY_LOCAL_MACHINE
-    elsif (key == 'HKCU' or key == 'HKEY_CURRENT_USER')
+    elsif (['HKCU', 'HKEY_CURRENT_USER'].include?(key))
       return HKEY_CURRENT_USER
-    elsif (key == 'HKU' or key == 'HKEY_USERS')
+    elsif (['HKU', 'HKEY_USERS'].include?(key))
       return HKEY_USERS
-    elsif (key == 'HKCR' or key == 'HKEY_CLASSES_ROOT')
+    elsif (['HKCR', 'HKEY_CLASSES_ROOT'].include?(key))
       return HKEY_CLASSES_ROOT
     elsif (key == 'HKEY_CURRENT_CONFIG')
       return HKEY_CURRENT_CONFIG

I've pulled down this pull request and run on the original file:

rubocop -a --only Style/MultipleComparison lib/rex/post/meterpreter/extensions/stdapi/sys/registry.rb

@koic koic force-pushed the fix_incorrect_autocorrect_for_style_multiple_comparison branch from 82f77d7 to 9ff9849 Compare February 16, 2021 18:00
@koic
Copy link
Member Author

koic commented Feb 16, 2021

@adfoster-r7 Thanks for trying this patch early! I have updated this PR with a newer patch.

@koic koic force-pushed the fix_incorrect_autocorrect_for_style_multiple_comparison branch 3 times, most recently from 585eac5 to 5e15ff6 Compare February 16, 2021 18:09
…omparison`

Fixes rubocop#9520.

This PR fixes an incorrect auto-correct for `Style/MultipleComparison`
when comparing a variable with multiple items in `if` and `elsif` conditions.
@koic koic force-pushed the fix_incorrect_autocorrect_for_style_multiple_comparison branch from 5e15ff6 to 80d6239 Compare February 16, 2021 18:37
@adfoster-r7
Copy link

Both scenarios work now, and I spotted no other issues after running the latest patch on the entire codebase - thanks! 👍

@bbatsov bbatsov merged commit d198e92 into rubocop:master Feb 19, 2021
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 19, 2021

Thanks!

@koic koic deleted the fix_incorrect_autocorrect_for_style_multiple_comparison branch February 19, 2021 13:00
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.

Style/MultipleComparison generates invalid code
3 participants