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 does not seem to like var nor method references #1956

Closed
maxandersen opened this issue May 25, 2021 · 6 comments · Fixed by eclipse-jdtls/eclipse.jdt.ls#1780
Closed
Assignees
Milestone

Comments

@maxandersen
Copy link
Contributor

maxandersen commented May 25, 2021

void somemethod() {
var gson = new GsonBuilder().setPrettyPrinting().create();
List<CatalogItem> aliasItems = new ArrayList<>();
List<CatalogItem> templateItems = new ArrayList<>();
PagedSearchIterable<GHContent> ghContents = gitHub.searchContent().filename("jbang-catalog.json").extension(".json")
        .list().withPageSize(500);
for (GHContent content : ghContents) {
      String location = content.getOwner().getFullName() + "/" + content.getPath();
      if (excludedCatalogs.contains(location)) {
        System.out.println("Excluded - " + location);
      } else {
        System.out.println("Processing - " + location);
        var catalogContent = toJsonElement(gson, content);
        if (catalogContent != null) {
          catalogContent.aliases.entrySet().stream().map(entry -> toCatalogerItem(entry, content))
              .forEach(aliasItems::add);

          catalogContent.templates.entrySet().stream().map(entry -> templateToItem(entry, content))
              .forEach(templateItems::add);
        }
      }
    }
}
}

gets refactored into this:

void somemethod() {
var gson = new GsonBuilder().setPrettyPrinting().create();
    List<CatalogItem> aliasItems = new ArrayList<>();
    List<CatalogItem> templateItems = new ArrayList<>();
    PagedSearchIterable<GHContent> ghContents = gitHub.searchContent().filename("jbang-catalog.json").extension(".json")
        .list().withPageSize(500);
    addContent(gson, ghContents);

}

private void addContent(var gson, PagedSearchIterable<GHContent> ghContents) throws IOException {
    for (GHContent content : ghContents) {
      String location = content.getOwner().getFullName() + "/" + content.getPath();
      if (excludedCatalogs.contains(location)) {
        System.out.println("Excluded - " + location);
      } else {
        System.out.println("Processing - " + location);
        var catalogContent = toJsonElement(gson, content);
        if (catalogContent != null) {
          catalogContent.aliases.entrySet().stream().map(entry -> toCatalogerItem(entry, content))
              .forEach(aliasItems::add);

          catalogContent.templates.entrySet().stream().map(entry -> templateToItem(entry, content))
              .forEach(templateItems::add);
        }
      }
    }
  }

notice how gson is of type var where it should be gson and neither aliasItems nor templateItems are present in the method declaration despite them being referenced.

@fbricon
Copy link
Collaborator

fbricon commented May 25, 2021

looks like your example is missing some steps. The ghContents line magically appeared in step 2

@maxandersen
Copy link
Contributor Author

for the var its easy to reproduce btw:

var test = new String("blah");
test = test + test;

extract method creates you:

private String extracted(var test) {
    return test + test;
  }

@fbricon
Copy link
Collaborator

fbricon commented May 25, 2021

ok. Not reproducible in Eclipse (which adds an intermediary step to list the variables and their type). So definitely.an issue in jdt.ls.

@snjeza
Copy link
Contributor

snjeza commented May 25, 2021

#1780 fixes the issue with the var

neither aliasItems nor templateItems are present in the method declaration despite them being referenced

This is an upstream issue.
Reproducible case:

public void foo() {
        var test = new String("t");
        List<String> list = new ArrayList<>();
        Arrays.asList("a", "b").forEach(list::add);
        test = test + test;
    }

try to extract

        Arrays.asList("a", "b").forEach(list::add);
        test = test + test;

Result (with the PR):

private void extracted(String test) {
        Arrays.asList("a", "b").forEach(list::add);
        test = test + test;
    }

The issue is also reproducible on Eclipse.

@maxandersen
Copy link
Contributor Author

did you open/find a upstream eclipse bug?

@rgrunber
Copy link
Member

did you open/find a upstream eclipse bug?

https://bugs.eclipse.org/bugs/show_bug.cgi?id=535638

They resolved it a while back, which makes our changes a bit easier to do. We inherited the helper methods (from shared components), but just didn't have the one-line changes to use them (since the refactorings themselves are in ui components)

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

Successfully merging a pull request may close this issue.

4 participants