Skip to content

Commit 5f03291

Browse files
committed
fix: fixes substitution issue with classpaths
Some normal options like "plugin:apply" were taken to be a classpath and where marked for substitution. We now properly ignore them. Fixes #68
1 parent ed9c8c4 commit 5f03291

File tree

3 files changed

+50
-19
lines changed

3 files changed

+50
-19
lines changed

app.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ dependencies:
1010
org.slf4j:slf4j-simple: "2.0.17"
1111

1212
actions:
13-
clean: ".{/}mvnw clean"
14-
build: ".{/}mvnw spotless:apply package -DskipTests"
15-
run: "{./target/binary/bin/jpm}"
16-
test: ".{/}mvnw test"
13+
clean: "./mvnw clean"
14+
build: "./mvnw spotless:apply package -DskipTests"
15+
run: "./target/binary/bin/jpm"
16+
test: "./mvnw test"
1717
buildj: "javac -cp {{deps}} -d classes --source-path src/main/java src/main/java/org/codejive/jpm/Main.java"
1818
runj: "java -cp classes:{{deps}} org.codejive.jpm.Main"

src/main/java/org/codejive/jpm/util/ScriptUtils.java

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,7 @@ public static int executeScript(String command, List<Path> classpath, boolean ve
3737
(isWindows() && tmpCommand.length() > 8000)
3838
|| (!isWindows() && tmpCommand.length() > 32000);
3939

40-
if (!usingSubstitutions(command)) {
41-
// First try to parse the command
42-
CommandsParser parser = new CommandsParser(command);
43-
CommandsParser.Commands commands = parser.parse();
44-
if (commands != null) {
45-
command = suggestSubstitutions(commands);
46-
}
47-
}
40+
command = suggestSubstitutions(command);
4841

4942
try (ArgsFiles argsFiles = new ArgsFiles()) {
5043
// Process the command for variable substitution and path conversion
@@ -73,21 +66,33 @@ public static String quoteArgument(String arg) {
7366
return arg;
7467
}
7568

69+
static String suggestSubstitutions(String command) {
70+
if (!usingSubstitutions(command)) {
71+
// First try to parse the command
72+
CommandsParser parser = new CommandsParser(command);
73+
CommandsParser.Commands commands = parser.parse();
74+
if (commands != null) {
75+
command = suggestCommandsSubstitutions(commands);
76+
}
77+
}
78+
return command;
79+
}
80+
7681
// Is the command using any substitutions? (we ignore {{deps}} here)
7782
private static boolean usingSubstitutions(String command) {
7883
command = command.replace("{]}", "\u0007");
7984
Pattern pattern = Pattern.compile("\\{([/:;~]|\\./.*|~/.*)}|@\\[.*]");
8085
return pattern.matcher(command).find();
8186
}
8287

83-
static String suggestSubstitutions(CommandsParser.Commands commands) {
88+
private static String suggestCommandsSubstitutions(CommandsParser.Commands commands) {
8489
StringBuilder cmd = new StringBuilder();
8590
for (CommandsParser.Node node : commands.elements) {
8691
if (node instanceof CommandsParser.Command) {
87-
cmd.append(suggestSubstitutions((CommandsParser.Command) node));
92+
cmd.append(suggestCommandSubstitutions((CommandsParser.Command) node));
8893
} else if (node instanceof CommandsParser.Group) {
8994
CommandsParser.Group group = (CommandsParser.Group) node;
90-
String groupStr = suggestSubstitutions(group.commands);
95+
String groupStr = suggestCommandsSubstitutions(group.commands);
9196
cmd.append("(").append(groupStr).append(")");
9297
} else if (node instanceof CommandsParser.Separator) {
9398
CommandsParser.Separator sep = (CommandsParser.Separator) node;
@@ -101,7 +106,7 @@ static String suggestSubstitutions(CommandsParser.Commands commands) {
101106
return cmd.toString();
102107
}
103108

104-
static String suggestSubstitutions(CommandsParser.Command command) {
109+
static String suggestCommandSubstitutions(CommandsParser.Command command) {
105110
StringBuilder cmd = new StringBuilder();
106111
if (command.words.isEmpty()) {
107112
return "";
@@ -179,12 +184,30 @@ private static String suggestClassPathSubstitution(String classpath) {
179184
if (parts.length <= 1) {
180185
return null;
181186
}
187+
// First see if this plausibly looks like a classpath
188+
// This requires at least one part to be a relative path with
189+
// at least two parts (i.e. have at least one / in it)
190+
// This does mean that it won't suggest substitutions for
191+
// classpaths that are all single-name parts (e.g. "lib:ext").
192+
if (!classpath.contains("{{deps}}")) {
193+
boolean hasAtLeastOneComplexPart = false;
194+
for (String path : parts) {
195+
Path p = FileUtils.safePath(path);
196+
if (p == null || p.isAbsolute()) {
197+
return null;
198+
}
199+
if (p.getNameCount() >= 2) {
200+
hasAtLeastOneComplexPart = true;
201+
}
202+
}
203+
if (!hasAtLeastOneComplexPart) {
204+
return null;
205+
}
206+
}
207+
// Now do the actual conversion
182208
StringBuilder sb = new StringBuilder();
183209
for (String path : parts) {
184210
Path p = FileUtils.safePath(path);
185-
if (p == null || p.isAbsolute()) {
186-
return null;
187-
}
188211
if (sb.length() == 0) {
189212
// Looks like a relative path, let's suggest {./...} or {~/...}
190213
if (p.startsWith("~")) {

src/test/java/org/codejive/jpm/util/ScriptUtilsTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,12 @@ void testProcessMultiCommandArgsFilesUse() throws Exception {
257257
assertThat(argsFiles.files.get(1)).hasContent(expectedContents2);
258258
}
259259
}
260+
261+
@Test
262+
void testProcessCommandWithInvalidClasspath() {
263+
String command = "./mvnw spotless:apply package -DskipTests";
264+
String result = ScriptUtils.suggestSubstitutions(command);
265+
// Substitutions should not have treated spottless:apply as a classpath
266+
assertThat(result).isEqualTo("{./mvnw} spotless:apply package -DskipTests");
267+
}
260268
}

0 commit comments

Comments
 (0)