Repository: nicoespeon/abracadabra
Pull request #876 (2023-05-15 18:12:07)

add new test for function refactoring with new lines
I just installed this extension, and notice the function refactor does not work in some cases.

I add a unit test to emphasis this behavior, which I consider being a bug.

The case is simple: when a function call statement contains new lines (example: one argument on its each line), the refactor (re-order args or remove arg) does not work.

## Steps to reproduce the behavior:

1. Given the following code:

```js
function /*[cursor]*/doWhatever(prefix, obj) {
    console.log(obj);
}

doWhatever("will be removed", "foo");

doWhatever(
    "will NOT be removed",
    "foo"
);
```

2. put cursor at `/*[cursor]*/` position
3. trigger "Change signature" tool
4. select `prefix` in the table shown
5. click "minus sign" to remove the `prefix` argument
6. click `confirm` to validate changes

## expected code

I expect:

1. the function declaration to be changed
2. the first function call to be changed
3. the second function call to be changed

```js
function /*[cursor]*/doWhatever(prefix, obj) {
    console.log(obj);
}

doWhatever("foo");

doWhatever(
    "foo"
);
```

## actual behavior

I get:

1. the function declaration is changed (OK)
2. the first function call is changed (OK)
3. the second function call is NOT changed (KO)

```js
function /*[cursor]*/doWhatever(prefix, obj) {
    console.log(obj);
}

doWhatever("foo");

doWhatever(
    "will NOT be removed",
    "foo"
);
```

Please note: the current pull request does not fix the bug as I don't have enough knowledge on this codebase to fix it.

I only added the unit test for the test I described. With theses changes, unit tests are now failling.


```bash
$ yarn test
// output skipped
Summary of all failing tests
 FAIL  src/refactorings/change-signature/change-signature.test.ts (9.496 s)
  ● Change Signature › In the same file with function declarations › when function call contains new lines

    expect(received).toBe(expected) // Object.is equality

    - Expected  - 1
    + Received  + 2

      function add(b, a) {
                  return a + b;
                }

    -           add(2, 1);
    +           add(1,
    +             2);

      215 |
      216 |       const extracted = await editor.codeOf(path);
    > 217 |       expect(extracted).toBe(expected);
          |                         ^
      218 |     }
      219 |   );
      220 |

      at toBe (src/refactorings/change-signature/change-signature.test.ts:217:25)


Test Suites: 1 failed, 68 passed, 69 total
Tests:       1 failed, 1 skipped, 1270 passed, 1272 total
Snapshots:   0 total
Time:        22.402 s, estimated 24 s
Ran all test suites.
error Command failed with exit code 1.
```


Patch:
diff --git a/src/refactorings/change-signature/change-signature.ts b/src/refactorings/change-signature/change-signature.ts
index 7c305d46..47887c44 100644
--- a/src/refactorings/change-signature/change-signature.ts
+++ b/src/refactorings/change-signature/change-signature.ts
@@ -264,7 +264,7 @@ function createVisitorForReferences(
       const end = Position.fromAST(path.node.loc.end).putAtStartOfLine();
       const nodeSelection = Selection.fromPositions(start, end);
 
-      if (!selection.isSameLineThan(nodeSelection)) return;
+      if (!selection.start.isSameLineThan(nodeSelection.start)) return;
 
       onMatch(path);
     },
@@ -314,7 +314,7 @@ function hasChildWhichMatchesSelection(
       const end = Position.fromAST(childPath.node.loc.end).putAtStartOfLine();
       const nodeSelection = Selection.fromPositions(start, end);
 
-      if (!selection.isSameLineThan(nodeSelection)) return;
+      if (!selection.start.isSameLineThan(nodeSelection.start)) return;
 
       result = true;
       childPath.stop();


Test patch:
diff --git a/src/refactorings/change-signature/change-signature.test.ts b/src/refactorings/change-signature/change-signature.test.ts
index 70d3d7a8..7ee7b8b9 100644
--- a/src/refactorings/change-signature/change-signature.test.ts
+++ b/src/refactorings/change-signature/change-signature.test.ts
@@ -40,6 +40,21 @@ describe("Change Signature", () => {
 
           add(2, 1);`
       },
+      {
+        description: "when function call contains new lines",
+        code: `function [cursor]add(a, b) {
+            return a + b;
+          }
+
+          add(1,
+            2);`,
+        expected: `function add(b, a) {
+            return a + b;
+          }
+
+          add(2,
+            1);`
+      },
       {
         description:
           "only wanted function keeping contract of the rest of functions",