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

Extract Method creates an unnecessary parameter for variable #59775

Open
DanTup opened this issue Dec 20, 2024 · 3 comments
Open

Extract Method creates an unnecessary parameter for variable #59775

DanTup opened this issue Dec 20, 2024 · 3 comments
Labels
analyzer-refactoring area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DanTup
Copy link
Collaborator

DanTup commented Dec 20, 2024

This was originally raised at Dart-Code/Dart-Code#5373 by @stephane-archer.

If you extract code that sets a variable, it becomes a return value for the extracted method. The original variable is also passed as an argument, even if it has never been read in the extracted code and could just be declared inside the new function.

For example:

void f() {
  int? i;
  i = 1; // extract this line
  print(i);
}

Becomes:

void f() {
  int? i;
  i = g(i);
  print(i);
}

int? g(int? i) {
  i = 1;
  return i;
}

But since i is not read by the extracted method (only written), the parameter could be dropped and instead the variable just declared inside the extracted function:

void f() {
  int? i;
  i = g();
  print(i);
}

int? g() {
  int? i = 1;
  return i;
}
@DanTup DanTup added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-refactoring type-enhancement A request for a change that isn't a bug labels Dec 20, 2024
@bwilkerson bwilkerson added the P3 A lower priority bug or feature request label Dec 20, 2024
@bwilkerson
Copy link
Member

The request is valid, but I suspect that it doesn't effect that many users. If I'm wrong, please upvote the issue as a signal.

@stephane-archer
Copy link

stephane-archer commented Dec 20, 2024

I think the code we extract can be much complex and this will create strange extracted code results.

Users might not recognize this pattern by looking at this simple example but they would see a lot of extra added parameters when they extract code.

I suspect this affects a lot of users but they just don't expect the code extractor tool to be smart and are okay with rewriting this after the tool has done a first step

@lrhn
Copy link
Member

lrhn commented Dec 20, 2024

(Independently of that parameter, it would be nice if the return type of the function was int, since that is the type of i after the expression has been evaluated.
Otherwise:

int? i;
i = 1;
print(i.toRadixString(16));

won't be able to convert the i = 1 to

 ...
 i = g(i);
...

int g(int? i) {
  i = 1;
  return i;
}

But that'd be another issue, so this is just a drive-by comment.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-refactoring area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants