Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

New rule prefer-object-spread #2624

Merged
merged 4 commits into from May 17, 2017
Merged

New rule prefer-object-spread #2624

merged 4 commits into from May 17, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Apr 21, 2017

PR checklist

Overview of change:

[new-rule] prefer-object-spread
Fixes: #2618
/cc @danvk

Is there anything you'd like reviewers to focus on?

I'm not sure if functionality is the right type in the metadata.

CHANGELOG.md entry:

[new-rule] `prefer-object-spread`
Fixes: #2618
@danvk
Copy link
Contributor

danvk commented Apr 21, 2017

Looks great once you get the tests passing!

@ajafff
Copy link
Contributor Author

ajafff commented Apr 22, 2017

In order to get the tests passing, I needed to change the sorting of the replacements. That could be considered a breaking change, but I rather think this is a bug fix. The tests of all other rules still pass.

The sorting of the replacements should be refactored afterwards. It should take into account if a replacement adds, removes or replaces text and sort accordingly.

@@ -150,7 +150,7 @@ export class Replacement {

public static applyAll(content: string, replacements: Replacement[]) {
// sort in reverse so that diffs are properly applied
replacements.sort((a, b) => b.end - a.end);
replacements.sort((a, b) => b.end - a.end || b.start - a.start);
Copy link
Contributor

Choose a reason for hiding this comment

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

we shouldn't even be applying overlapping fixes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule suggestion: prefer object spread to Object.assign
4 participants