Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@
import picocli.CommandLine;

import java.io.IOException;
import java.io.Reader;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

public class AccessTransformersTransformer implements SourceTransformer {
Expand All @@ -25,15 +29,21 @@ public class AccessTransformersTransformer implements SourceTransformer {
@CommandLine.Option(names = "--access-transformer-validation", description = "The level of validation to use for ats")
public AccessTransformerValidation validation = AccessTransformerValidation.LOG;

// TODO: as is, either this or the normal option is required
@CommandLine.Option(names = "--access-transformer-no-validation")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--access-transformer-validation-exclude ??

public List<Path> atFilesWithoutValidation;

private AccessTransformerFiles ats;
private Map<Target, Transformation> pendingATs;
private Set<String> sourcesWithoutValidation;
private Logger logger;
private volatile boolean errored;

@Override
public void beforeRun(TransformContext context) {
ats = new AccessTransformerFiles();
logger = context.logger();
sourcesWithoutValidation = new HashSet<>();

for (Path path : atFiles) {
try {
Expand All @@ -44,20 +54,44 @@ public void beforeRun(TransformContext context) {
}
}

for (Path path : atFilesWithoutValidation) {
var sourceName = path.toAbsolutePath().toString();
try (Reader reader = Files.newBufferedReader(path)) {
ats.loadAT(reader, sourceName);
} catch (IOException ex) {
logger.error("Failed to parse access transformer file %s: %s", path, ex.getMessage());
throw new UncheckedIOException(ex);
}
sourcesWithoutValidation.add(sourceName);
}

pendingATs = new ConcurrentHashMap<>(ats.getAccessTransformers());
}

@Override
public boolean afterRun(TransformContext context) {
if (!pendingATs.isEmpty()) {
pendingATs.forEach((target, transformation) -> {
// ClassTarget for inner classes have a corresponding InnerClassTarget which is more obvious for users
// so we don't log the ClassTarget as that will cause duplication
if (target instanceof Target.ClassTarget && target.className().contains("$")) return;
logger.error("Access transformer %s, targeting %s did not apply as its target doesn't exist", transformation, target);
});
pendingATs.forEach((target, transformation) -> {
// Ignore targets that could not be applied,
// but are only referenced in AT files for which we don't want to perform any validation
boolean ignore = transformation.origins()
.stream()
.allMatch(origin -> {
var colonPos = origin.lastIndexOf(':');
var sourceName = colonPos == -1 ? origin : origin.substring(0, colonPos);
return sourcesWithoutValidation.contains(sourceName);
});
if (ignore) {
return;
}
// ClassTarget for inner classes have a corresponding InnerClassTarget which is more obvious for users
// so we don't log the ClassTarget as that will cause duplication
if (target instanceof Target.ClassTarget && target.className().contains("$")) {
return;
}

logger.error("Access transformer %s, targeting %s did not apply as its target doesn't exist", transformation, target);
errored = true;
}
});

return !(errored && validation == AccessTransformerValidation.ERROR);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public C1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public Cdoesnotexist
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public class C1 {
C1() {}
}
2 changes: 2 additions & 0 deletions tests/data/accesstransformer/partially_invalid/source/C1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class C1 {
}
23 changes: 23 additions & 0 deletions tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,29 @@ void testIllegal() throws Exception {
void testFolderClasspathEntries() throws Exception {
runATTest("folder_classpath_entry", "--classpath=" + testDataRoot.resolve("accesstransformer/folder_classpath_entry/deps"));
}

@Test
void testPartialValidation() throws Exception {
var testDirName = "accesstransformer/partially_invalid";
var checkedAtPath = testDataRoot.resolve(testDirName).resolve("accesstransformer-checked.cfg");
var uncheckedAtPath = testDataRoot.resolve(testDirName).resolve("accesstransformer-unchecked.cfg");
runTest(testDirName, txt -> txt,
"--enable-accesstransformers",
"--access-transformer", checkedAtPath.toString(),
"--access-transformer-no-validation", uncheckedAtPath.toString(),
"--access-transformer-validation=error"
);

// Swapping which file is validated should throw:
assertThrows(RuntimeException.class, () -> {
runTest(testDirName, txt -> txt,
"--enable-accesstransformers",
"--access-transformer", uncheckedAtPath.toString(),
"--access-transformer-no-validation", checkedAtPath.toString(),
"--access-transformer-validation=error"
);
});
}
}

@Nested
Expand Down
Loading