Suggestions 🎉

[sunny]

When enabling suggestions: true in the Pronto YML configuration under rubocop:, messages will include a line suggesting what to change, using GitHub's new syntax on Pull Request reviews.

For example, for this change:

  end
+
+def do_something
+  "bar"
+end
end

The message will have a suggestion in this format:

Prefer single-quoted strings when you don't need string interpolation
or special symbols.

```suggestion
  'bar'
```

Which appears as GitHub comments like this:

That can be applied in one click right from the Pull Request.

[mmozuras]

@sunny looks good. Could you rebase this PR on top of current master?

[sunny]

Hey @mmozuras, thanks for looking into this! I rebased against master.

---

I am using the RUBOCOP_SUGGESTION_COPS ENV var in my CI and for my use-case, this PR works fine and could be merged as-is.

However for the long term, I am not convinced that using an environment variable is the best way to store this configuration. Would you have any better suggestions as to where to hold this list of suggestable cops?

I am considering:

• An option in .pronto.yml, e.g.

  rubocop:
    suggestion_cops:
      - Style/StringLiterals
      - Style/TrailingCommaInArrayLiteral
      - Style/TrailingCommaInHashLiteral
      - Style/TrailingCommaInArguments
  

  => This would need to come in addition to the env PRONTO_RUBOCOP_SUGGESTION_COPS (as a comma-seperated string instead) if I believe pronto's README.

• Configuration inside rubocop.yml
  => that would require to do modifications inside RuboCop itself

• Or, perhaps, having a single "show suggestions" setting and using another heuristic to determine whether this is a cop that works well as a suggestion (does it have autocorrect, and if yes did it only touch one line, for example).

• This could open up another option: having it as a variable inside the GitHub formatter in .pronto.yml instead of a "show suggestions" setting.

  github_pr:
    format: "%{runner} - %{msg}%{suggestion}"

  => I don't know how much that would be preferable.

[mmozuras]

@sunny I like the idea of putting it somewhere/somehow in .pronto.yml. The first option you've suggested looks good. I think that the solution could also be generalized for each Pronto runner to have access to it.

@doomspork what do you think?

[doomspork]

Howdy @sunny and @mmozuras! I like the cleanliness of Option 1 👌

@prontolabs/core any additional thoughts?

[aergonaut]

I believe GitHub suggestions only work with single-line diffs. What would happen if the RuboCop suggestion is longer than 1 line? Maybe there should be a check to make sure that only 1-line corrections get turned into suggestions, and other corrections just get included as code blocks?

[sunny]

@aergonaut

Maybe there should be a check to make sure that only 1-line corrections get turned into suggestions, and other corrections just get included as code blocks?

That would be awesome, we wouldn't need to keep a list of "safe cops" at all that way.

However, I didn't manage to get it working that way since calling all cops with autocorrect can change the lines of the next suggestions. Maybe I need to continue digging to find a good solution. (Or if anybody has an idea on how to tackle this, would love some help.)

[sunny]

@aergonaut @doomspork @mmozuras I figured out how to only show suggestions of cops that moved only one line. This means we don't need an environment variable that lists all cops, but just a configuration setting to enable/disable suggestions. \o/

What do you think?

[doomspork]

@sunny sorry for the delay, that single option sounds ideal 👍

I'll review your PR and add the @prontolabs/core team as well for a final review.

[sunny]

Any chance for a final review here? 🙏

[doomspork]

@sunny sorry for the delays, could you rebase so I can merge?

[sunny]

@doomspork Rebased! I fixed a few conflicts over the new configuration for severity.

[sunny]

Woohoo! \o/ Thanks for the review and the merge. 🎉