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.

Sign up for GitHub

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

Jump to bottom

[FIX #7777, #7776]: Generate valid code if comment exists before closing brace #7858

Closed
wants to merge 1 commit into from
Closed

[FIX #7777, #7776]: Generate valid code if comment exists before closing brace #7858

wants to merge 1 commit into from

Conversation

shekhar-patil
Copy link
Contributor

@shekhar-patil shekhar-patil commented Apr 7, 2020 •
edited

closes #7777 and #7776

Before submitting the PR make sure the following are checked:

  • 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.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@shekhar-patil shekhar-patil changed the title [FIX-7777-7776]: Generate valid code if comment exists before closing b… [FIX #7777, #7776]: Generate valid code if comment exists before closing brace Apr 7, 2020
mwolman
mwolman reviewed Apr 8, 2020
View reviewed changes
spec/rubocop/cli/cli_autocorrect_spec.rb Outdated
@@ -945,6 +945,44 @@ class Bar
RUBY
end

it 'correct multiline array brace without crashing' do
Copy link

Choose a reason for hiding this comment

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

Suggested change
it 'correct multiline array brace without crashing' do
it 'corrects multiline array brace without crashing' do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

spec/rubocop/cli/cli_autocorrect_spec.rb Outdated
second_key: 'value c'
}
RUBY
expect(cli.run(['--auto-correct'])).to eq(1)
Copy link

Choose a reason for hiding this comment

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

Suggested change
expect(cli.run(['--auto-correct'])).to eq(1)
expect(cli.run(['--auto-correct'])).to eq(1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

spec/rubocop/cli/cli_autocorrect_spec.rb Outdated
RUBY
end

it 'correct multiline literal brace without crashing' do
Copy link

Choose a reason for hiding this comment

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

Suggested change
it 'correct multiline literal brace without crashing' do
it 'corrects multiline literal brace without crashing' do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

spec/rubocop/cli/cli_autocorrect_spec.rb Outdated
'hash' => {'key' => 'value'} # comment
))
RUBY
expect(cli.run(['--auto-correct'])).to eq(0)
Copy link

Choose a reason for hiding this comment

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

Suggested change
expect(cli.run(['--auto-correct'])).to eq(0)
expect(cli.run(['--auto-correct'])).to eq(0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch from 4099bbf to 6549a24 Compare April 9, 2020 04:39
@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch from 6549a24 to 313df45 Compare April 10, 2020 14:55
bbatsov
bbatsov reviewed Apr 10, 2020
View reviewed changes
spec/rubocop/cli/cli_autocorrect_spec.rb Outdated
@@ -945,6 +945,46 @@ class Bar
RUBY
end

it 'corrects multiline array brace without crashing' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why didn't you add those tests to the spec of the cop instead? It seems to me this is the natural place for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bbatsov PR is ready for review.

@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch 3 times, most recently from a2485a1 to 344fd63 Compare April 13, 2020 14:24
bbatsov
bbatsov reviewed Apr 15, 2020
View reviewed changes
lib/rubocop/cop/correctors/multiline_literal_brace_corrector.rb Outdated
)
end

def closing_brace_content(corrector, node)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the names closing_brace_content and trailing_content_of_closing_brace are a bit confusing, as braces can't really have content. The naming should be improved.

Copy link

Choose a reason for hiding this comment

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

@bbatsov changes done. Please review now.

@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch from 344fd63 to b92004c Compare April 16, 2020 07:38
@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch 3 times, most recently from b2e1945 to 60598a5 Compare April 16, 2020 08:46
@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch from 60598a5 to e9c2c2a Compare April 24, 2020 06:40
@shekhar-patil shekhar-patil force-pushed the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch from e9c2c2a to de65e13 Compare April 24, 2020 07:10
@shekhar-patil
Copy link
Contributor Author

@bbatsov this is kind reminder to you. Please review and let me know if any changes required.

@shekhar-patil shekhar-patil deleted the Fix-7777-7776-Generate-valid-code-if-comment-exists-before-closing-brace branch July 4, 2020 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shekhar-amura shekhar-amura shekhar-amura left review comments

@mwolman mwolman mwolman approved these changes

@bbatsov bbatsov Awaiting requested review from bbatsov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rubocop places commas into comments, generating invalid ruby code
4 participants
@shekhar-patil @bbatsov @mwolman @shekhar-amura