Skip to content

Commit

Permalink
fix for issue helidon-io#7081
Browse files Browse the repository at this point in the history
  • Loading branch information
@trentjeff
trentjeff committed Jul 6, 2023
1 parent a61a942 commit bd8fcf1
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 62 deletions.
2 changes: 1 addition & 1 deletion pico/tests/resources-pico/src/test/resources/expected/tests-module-info.java._pico_
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// @io.helidon.common.Generated(value = "io.helidon.pico.tools.ModuleInfoDescriptor", trigger = "io.helidon.pico.tools.ModuleInfoDescriptor")
// @io.helidon.common.Generated(value = "io.helidon.pico.tools.ActivatorCreatorDefault", trigger = "io.helidon.pico.tools.ActivatorCreatorDefault")
module io.helidon.pico.tests.pico/test {
exports io.helidon.pico.tests.pico;
// Pico module - @io.helidon.common.Generated(value = "io.helidon.pico.tools.ActivatorCreatorDefault", trigger = "io.helidon.pico.tools.ActivatorCreatorDefault")
Expand Down
32 changes: 30 additions & 2 deletions pico/tools/src/main/java/io/helidon/pico/tools/ModuleInfoDescriptorBlueprint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,30 @@ interface ModuleInfoDescriptorBlueprint {
@Prototype.Singular
List<ModuleInfoItem> items();

/**
* The items that were not handled (due to parsing outages, etc.).
*
* @return the list of unhandled lines
*/
@Prototype.Singular
List<String> unhandledLines();

/**
* Any throwable/errors that were encountered during parsing.
*
* @return optionally any error encountered during parsing
*/
Optional<Throwable> error();

/**
* Returns {@code true} if there were no cases of {@link #unhandledLines()} or {@link #error()}'s encountered.
*
* @return true if any parsing of the given module-info descriptor appears to be full and complete
*/
default boolean handled() {
return error().isEmpty() && unhandledLines().isEmpty();
}

/**
* Returns true if the name currently set is the same as the {@link #DEFAULT_MODULE_NAME}.
*
Expand Down Expand Up @@ -218,8 +242,12 @@ default String contents(boolean wantAnnotation) {
descriptionComment().ifPresent(it -> subst.put("description", it));
subst.put("hasdescription", descriptionComment().isPresent());
String template = helper.safeLoadTemplate(templateName(), ModuleUtils.SERVICE_PROVIDER_MODULE_INFO_HBS);
String contents = helper.applySubstitutions(template, subst, true).trim();
return CommonUtils.trimLines(contents);
String contents = helper.applySubstitutions(template, subst, true);
contents = CommonUtils.trimLines(contents);
if (!wantAnnotation) {
contents = contents.replace("\t", " ");
}
return contents;
}

}
74 changes: 24 additions & 50 deletions pico/tools/src/main/java/io/helidon/pico/tools/ModuleInfoDescriptorSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static ModuleInfoDescriptor create(Path path,
ModuleInfoOrdering ordering) {
try {
String moduleInfo = Files.readString(path);
return create(moduleInfo, ordering);
return create(moduleInfo, ordering, false);
} catch (IOException e) {
throw new ToolsException("Unable to load: " + path, e);
}
Expand All @@ -75,20 +75,22 @@ static ModuleInfoDescriptor create(Path path,
*/
@Prototype.FactoryMethod
static ModuleInfoDescriptor create(String moduleInfo) {
return create(moduleInfo, ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS);
return create(moduleInfo, ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS, false);
}

/**
* Loads and creates the {@code module-info} descriptor given its literal source and preferred ordering scheme.
*
* @param moduleInfo the source
* @param ordering the ordering to apply
* @param strict when set to true, parsing must pass fully and completely (i.e., {@link ModuleInfoDescriptor#handled()} ()} must be true)
* @return the module-info descriptor
* @throws io.helidon.pico.tools.ToolsException if there is any exception encountered
*/
@Prototype.FactoryMethod
static ModuleInfoDescriptor create(String moduleInfo,
ModuleInfoOrdering ordering) {
ModuleInfoOrdering ordering,
boolean strict) {
ModuleInfoDescriptor.Builder descriptor = ModuleInfoDescriptor.builder();

String clean = moduleInfo;
Expand All @@ -99,9 +101,6 @@ static ModuleInfoDescriptor create(String moduleInfo,
clean = moduleInfo.replaceAll("/\\*[^*]*(?:\\*(?!/)[^*]*)*\\*/|//.*", "");
}

// remove annotations
clean = cleanModuleAnnotations(clean);

boolean firstLine = true;
Map<String, TypeName> importAliases = new LinkedHashMap<>();
String line = null;
Expand Down Expand Up @@ -158,7 +157,19 @@ static ModuleInfoDescriptor create(String moduleInfo,
}
descriptor.addItem(provides.build());
} else if (line.startsWith("opens ")) {
// follow up issue
ModuleInfoItem.Builder opens = ModuleInfoItem.builder()
.opens(true)
.target(resolve(split[1], importAliases))
.precomments((comments != null) ? comments : List.of());
if (split.length < 3) {
throw new ToolsException("Unable to process module-info's use of: " + line);
}
if (split[2].equals("to")) {
for (int i = 3; i < split.length; i++) {
opens.addWithOrTo(cleanLine(split[i]));
}
}
descriptor.addItem(opens.build());
} else if (line.equals("}")) {
break;
} else {
Expand All @@ -169,14 +180,14 @@ static ModuleInfoDescriptor create(String moduleInfo,
comments = new ArrayList<>();
}
}
} catch (ToolsException e) {
throw e;
} catch (Exception e) {
ToolsException te;
if (line != null) {
throw new ToolsException("Failed on line: " + line + ";\n"
+ "unable to load or parse module-info: " + moduleInfo, e);
e = new ToolsException("Failed to parse line: " + line, e);
descriptor.addUnhandledLine(line);
}
throw new ToolsException("Unable to load or parse module-info: " + moduleInfo, e);
te = new ToolsException("Unable to load or parse module-info: " + moduleInfo, e);
descriptor.error(te);
}

return descriptor.build();
Expand Down Expand Up @@ -207,7 +218,7 @@ static ModuleInfoDescriptor create(InputStream is,
ModuleInfoOrdering ordering) {
try {
String moduleInfo = new String(is.readAllBytes(), StandardCharsets.UTF_8);
return create(moduleInfo, ordering);
return create(moduleInfo, ordering, false);
} catch (IOException e) {
throw new ToolsException("Unable to load from stream", e);
}
Expand Down Expand Up @@ -236,43 +247,6 @@ private static ModuleInfoItem requiresModuleName(String moduleName,
.build();
}

private static String cleanModuleAnnotations(String moduleText) {
StringBuilder response = new StringBuilder();

try (BufferedReader br = new BufferedReader(new StringReader(moduleText))) {
boolean inModule = false;
String line;
while ((line = br.readLine()) != null) {
if (inModule) {
response.append(line).append("\n");
continue;
}
String trimmed = line.trim();
if (trimmed.startsWith("/*")) {
// beginning of comments
response.append(line).append("\n");
} else if (trimmed.startsWith("*")) {
// comment line
response.append(line).append("\n");
} else if (trimmed.startsWith("import ")) {
// import line
response.append(line).append("\n");
} else if (trimmed.startsWith("module")) {
// now just add the rest (we do not cover annotations within module text)
inModule = true;
response.append(line).append("\n");
} else if (trimmed.isBlank()) {
// empty line
response.append("\n");
}
}
} catch (IOException ignored) {
// ignored, we cannot get an exception when closing string reader
}

return response.toString();
}

private static String cleanLine(String line) {
if (line == null) {
return null;
Expand Down
5 changes: 5 additions & 0 deletions pico/tools/src/main/java/io/helidon/pico/tools/ModuleInfoItemBlueprint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ default String contents() {
assert (!isStaticUsed());
assert (!exports());
builder.append("opens ");
if (!withOrTo().isEmpty()) {
builder.append(Objects.requireNonNull(target()));
builder.append(" to ").append(String.join(",\n\t\t\t", withOrToSorted()));
return builder.toString();
}
handled = true;
}
if (requires()) {
Expand Down
60 changes: 51 additions & 9 deletions pico/tools/src/test/java/io/helidon/pico/tools/ModuleInfoDescriptorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -146,16 +147,47 @@ void sortedWithComments() {
@Test
void innerCommentsNotSupported() {
String moduleInfo = "module test {\nprovides /* inner comment */ cn;\n}";
ToolsException te = assertThrows(ToolsException.class, () -> ModuleInfoDescriptor.create(moduleInfo));
assertThat(te.getMessage(),
equalTo("Unable to parse lines that have inner comments: 'provides /* inner comment */ cn'"));
ModuleInfoDescriptor descriptor = ModuleInfoDescriptor.create(moduleInfo);
assertThat(descriptor.handled(),
is(false));
assertThat(descriptor.unhandledLines(),
contains("module test {"));
assertThat(descriptor.error().orElseThrow().getMessage(),
equalTo("Unable to load or parse module-info: module test {\nprovides /* inner comment */ cn;\n}"));
}

// filed https://github.com/helidon-io/helidon/issues/5697
@Test
void annotationsNotSupported() {
String moduleInfo = "import io.helidon.common.features.api.Feature;\n"
+ "import io.helidon.common.features.api.HelidonFlavor;\n"
+ "\n"
+ "/**\n"
+ " * Helidon SE Config module.\n"
+ " *\n"
+ " * @see io.helidon.config\n"
+ " */\n"
+ "@Feature(value = \"Config\",\n"
+ " description = \"Configuration module\",\n"
+ " in = {HelidonFlavor.NIMA, HelidonFlavor.SE}\n"
+ ")\n"
+ "module io.helidon.config {\n}\n";
ModuleInfoDescriptor descriptor = ModuleInfoDescriptor.create(moduleInfo);
assertThat(descriptor.handled(),
is(false));
assertThat(descriptor.unhandledLines(),
contains("@Feature(value = \"Config\", description = \"Configuration module\", in = {HelidonFlavor"
+ ".NIMA, HelidonFlavor.SE}"));
assertThat(descriptor.error().orElseThrow().getCause().getMessage(),
equalTo("Failed to parse line: @Feature(value = \"Config\", description = \"Configuration module\", "
+ "in = {HelidonFlavor.NIMA, HelidonFlavor.SE}"));
}

@Test
void loadCreateAndSave() throws Exception {
ModuleInfoDescriptor descriptor = ModuleInfoDescriptor
.create(CommonUtils.loadStringFromResource("testsubjects/m0._java_"),
ModuleInfoOrdering.NATURAL);
ModuleInfoOrdering.NATURAL, true);
assertThat(descriptor.contents(false),
equalTo("module io.helidon.pico {\n"
+ " requires transitive io.helidon.pico.api;\n"
Expand All @@ -167,10 +199,12 @@ void loadCreateAndSave() throws Exception {
+ ".DefaultPicoServices;\n"
+ " uses io.helidon.pico.api.ModuleComponent;\n"
+ " uses io.helidon.pico.api.Application;\n"
+ " opens io.helidon.config to weld.core.impl,\n"
+ " io.helidon.microprofile.cdi;\n"
+ "}"));

String contents = CommonUtils.loadStringFromFile("target/test-classes/testsubjects/m0._java_").trim();
descriptor = ModuleInfoDescriptor.create(contents, ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS);
descriptor = ModuleInfoDescriptor.create(contents, ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS, true);
assertThat(descriptor.contents(false),
equalTo(contents));

Expand All @@ -182,7 +216,7 @@ void loadCreateAndSave() throws Exception {
String contents2 = CommonUtils.loadStringFromFile("target/test-classes/testsubjects/m0._java_").trim();
assertThat(contents, equalTo(contents2));
ModuleInfoDescriptor descriptor2 =
ModuleInfoDescriptor.create(contents, ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS);
ModuleInfoDescriptor.create(contents, ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS, true);
assertThat(descriptor, equalTo(descriptor2));
} finally {
if (tempFile != null) {
Expand All @@ -195,18 +229,26 @@ void loadCreateAndSave() throws Exception {
void mergeCreate() {
ModuleInfoDescriptor descriptor = ModuleInfoDescriptor
.create(CommonUtils.loadStringFromResource("testsubjects/m0._java_"),
ModuleInfoOrdering.NATURAL);
ModuleInfoOrdering.NATURAL_PRESERVE_COMMENTS, true);
assertThat(descriptor.contents(false),
equalTo("module io.helidon.pico {\n"
+ "\n"
+ " requires transitive io.helidon.pico.api;\n"
+ " requires static com.fasterxml.jackson.annotation;\n"
+ " requires static lombok;\n"
+ " requires io.helidon.common;\n"
+ "\n"
+ " exports io.helidon.pico.spi.impl;\n"
+ " provides io.helidon.pico.api.PicoServices with io.helidon.pico.spi.impl"
+ ".DefaultPicoServices;\n"
+ "\n"
+ " provides io.helidon.pico.api.PicoServices with io.helidon.pico.spi.impl.DefaultPicoServices;\n"
+ "\n"
+ " uses io.helidon.pico.api.ModuleComponent;\n"
+ " uses io.helidon.pico.api.Application;\n"
+ "\n"
+ " // needed when running with modules - to make private methods accessible\n"
+ " // another comment with a semicolon;\n"
+ " opens io.helidon.config to weld.core.impl,\n"
+ " io.helidon.microprofile.cdi;\n"
+ "}"));
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> descriptor.mergeCreate(descriptor));
assertThat(e.getMessage(), equalTo("can't merge with self"));
Expand Down
5 changes: 5 additions & 0 deletions pico/tools/src/test/resources/testsubjects/m0._java_
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,9 @@ module io.helidon.pico {

uses io.helidon.pico.api.ModuleComponent;
uses io.helidon.pico.api.Application;

// needed when running with modules - to make private methods accessible
// another comment with a semicolon;
opens io.helidon.config to weld.core.impl,
io.helidon.microprofile.cdi;
}

0 comments on commit bd8fcf1

Please sign in to comment.