Skip to content

8368848: JShell's code completion not always working for multi-snippet inputs #27552

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

8368848: JShell's code completion not always working for multi-snippet inputs #27552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/jdk.jshell/share/classes/jdk/jshell/OuterWrapMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ private CompoundWrap wrappedInClass(String className, String imports, List<Wrap>

OuterWrap wrapInClass(Set<Key> except, Collection<Snippet> plus,
List<Snippet> snippets, List<Wrap> wraps) {
String imports = state.maps.packageAndImportsExcept(except, plus);
List<String> extraImports =
plus.stream()
.map(psi -> psi.importLine(state))
.toList();
String imports = state.maps.packageAndImportsExcept(except, extraImports);
// className is unique to the set of snippets and their version (seq)
String className = REPL_CLASS_PREFIX + snippets.stream()
.sorted((sn1, sn2) -> sn1.key().index() - sn2.key().index())
Expand All @@ -86,9 +90,16 @@ OuterWrap wrapInClass(Set<Key> except, Collection<Snippet> plus,
}

OuterWrap wrapInTrialClass(Wrap wrap) {
String imports = state.maps.packageAndImportsExcept(null, null);
return wrapInTrialClass(List.of(), List.of(), wrap);
}

OuterWrap wrapInTrialClass(List<String> extraImports, List<Wrap> preWraps, Wrap wrap) {
String imports = state.maps.packageAndImportsExcept(null, extraImports);
List<Wrap> allWraps = new ArrayList<>();
allWraps.addAll(preWraps);
allWraps.add(wrap);
CompoundWrap w = wrappedInClass(REPL_DOESNOTMATTER_CLASS_NAME, imports,
Collections.singletonList(wrap));
allWraps);
return new OuterWrap(w);
}

Expand Down
6 changes: 2 additions & 4 deletions src/jdk.jshell/share/classes/jdk/jshell/SnippetMaps.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,15 @@ List<Snippet> snippetList() {
return new ArrayList<>(snippets);
}

String packageAndImportsExcept(Set<Key> except, Collection<Snippet> plus) {
String packageAndImportsExcept(Set<Key> except, Collection<String> extraImports) {
StringBuilder sb = new StringBuilder();
sb.append("package ").append(REPL_PACKAGE).append(";\n");
for (Snippet si : keyIndexToSnippet) {
if (si != null && si.status().isDefined() && (except == null || !except.contains(si.key())) && si.name() != null && !si.name().isEmpty()) {
sb.append(si.importLine(state));
}
}
if (plus != null) {
plus.forEach(psi -> sb.append(psi.importLine(state)));
}
extraImports.forEach(sb::append);
return sb.toString();
}

Expand Down
118 changes: 99 additions & 19 deletions src/jdk.jshell/share/classes/jdk/jshell/SourceCodeAnalysisImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,17 +300,8 @@ private List<Suggestion> completionSuggestionsImpl(String code, int cursor, int[
identifier = m.group();
}
}
code = code.substring(0, cursor);
if (code.trim().isEmpty()) { //TODO: comment handling
code += ";";
}
boolean[] moduleImport = new boolean[1];
OuterWrap codeWrap = switch (guessKind(code, moduleImport)) {
case IMPORT -> moduleImport[0] ? proc.outerMap.wrapImport(Wrap.simpleWrap(code), null)
: proc.outerMap.wrapImport(Wrap.simpleWrap(code + "any.any"), null);
case CLASS, METHOD -> proc.outerMap.wrapInTrialClass(Wrap.classMemberWrap(code));
default -> proc.outerMap.wrapInTrialClass(Wrap.methodWrap(code));
};

OuterWrap codeWrap = wrapCodeForCompletion(code, cursor, true);
String[] requiredPrefix = new String[] {identifier};
return computeSuggestions(codeWrap, code, cursor, requiredPrefix, anchor).stream()
.filter(s -> filteringText(s).startsWith(requiredPrefix[0]) && !s.continuation().equals(REPL_DOESNOTMATTER_CLASS_NAME))
Expand Down Expand Up @@ -1740,15 +1731,11 @@ public List<Documentation> documentation(String code, int cursor, boolean comput
};

private List<Documentation> documentationImpl(String code, int cursor, boolean computeJavadoc) {
code = code.substring(0, cursor);
if (code.trim().isEmpty()) { //TODO: comment handling
code += ";";
}

if (guessKind(code) == Kind.IMPORT)
OuterWrap codeWrap = wrapCodeForCompletion(code, cursor, false);
if (codeWrap == null) {
//import:
return Collections.emptyList();

OuterWrap codeWrap = proc.outerMap.wrapInTrialClass(Wrap.methodWrap(code));
}
return proc.taskFactory.analyze(codeWrap, List.of(keepParameterNames), at -> {
SourcePositions sp = at.trees().getSourcePositions();
CompilationUnitTree topLevel = at.firstCuTree();
Expand Down Expand Up @@ -2429,6 +2416,99 @@ public static void waitCurrentBackgroundTasksFinished() throws Exception {
INDEXER.submit(() -> {}).get();
}

private OuterWrap wrapCodeForCompletion(String code, int cursor, boolean wrapImports) {
code = code.substring(0, cursor);
if (code.trim().isEmpty()) { //TODO: comment handling
code += ";";
}

List<String> imports = new ArrayList<>();
List<Wrap> declarationParts = new ArrayList<>();
String lastImport = null;
boolean lastImportIsModuleImport = false;
Wrap declarationWrap = null;
Wrap pendingWrap = null;
String input = code;
boolean cont = true;
int startOffset = 0;

while (cont) {
if (lastImport != null) {
imports.add(lastImport);
lastImport = null;
}
if (declarationWrap != null) {
declarationParts.add(declarationWrap);
declarationWrap = null;
pendingWrap = null;
}

String current;
SourceCodeAnalysis.CompletionInfo completeness = analyzeCompletion(input);
int newStartOffset;

if (completeness.completeness().isComplete() && !completeness.remaining().isBlank()) {
current = input.substring(0, input.length() - completeness.remaining().length());
newStartOffset = startOffset + input.length() - completeness.remaining().length();
input = completeness.remaining();
cont = true;
} else {
current = input;
cont = false;
newStartOffset = startOffset;
}

boolean[] moduleImport = new boolean[1];

switch (guessKind(current, moduleImport)) {
case IMPORT -> {
lastImport = current;
lastImportIsModuleImport = moduleImport[0];
}
case CLASS, METHOD -> {
pendingWrap = declarationWrap = Wrap.classMemberWrap(whitespaces(code, startOffset) + current);
}
case VARIABLE -> {
declarationWrap = Wrap.classMemberWrap(whitespaces(code, startOffset) + current);
pendingWrap = Wrap.methodWrap(whitespaces(code, startOffset) + current);
}
default -> {
pendingWrap = declarationWrap = Wrap.methodWrap(whitespaces(code, startOffset) + current);
}
}

startOffset = newStartOffset;
}

if (lastImport != null) {
if (wrapImports) {
return proc.outerMap.wrapImport(Wrap.simpleWrap(whitespaces(code, startOffset) + lastImport + (!lastImportIsModuleImport ? "any.any" : "")), null);
} else {
return null;
}
}

if (pendingWrap != null) {
return proc.outerMap.wrapInTrialClass(imports, declarationParts, pendingWrap);
}

throw new IllegalStateException("No pending wrap for: " + code);
}

private static String whitespaces(String input, int offset) {
StringBuilder result = new StringBuilder();

for (int i = 0; i < offset; i++) {
if (input.charAt(i) == '\n') {
result.append('\n');
} else {
result.append(' ');
}
}

return result.toString();
}

/**
* A candidate for continuation of the given user's input.
*/
Expand Down
10 changes: 10 additions & 0 deletions test/langtools/jdk/jshell/CompletionSuggestionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -941,4 +941,14 @@ public void testAnnotation() {
assertEval("import static java.lang.annotation.RetentionPolicy.*;");
assertCompletion("@AnnA(C|", true, "CLASS");
}

@Test
public void testMultiSnippet() {
assertCompletion("String s = \"\"; s.len|", true, "length()");
assertCompletion("String s() { return \"\"; } s().len|", true, "length()");
assertCompletion("String s() { return \"\"; } import java.util.List; List.o|", true, "of(");
assertCompletion("String s() { return \"\"; } import java.ut| ", true, "util.");
assertCompletion("class S { public int length() { return 0; } } new S().len|", true, "length()");
assertSignature("void f() { } f(|", "void f()");
}
}