Skip to content

Commit

Permalink
Merge pull request #15 from xzel23/feature/cabe14-supporting-throwing…
Browse files Browse the repository at this point in the history 
…-IAE

Feature/cabe14 supporting throwing iae
  • Loading branch information
@xzel23
xzel23 authored Nov 29, 2024
2 parents 2b66220 + 8d7c7e5 commit e8ab780
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 103 deletions.
9 changes: 9 additions & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version 3.0.1
=============

- introduce Check.THROW_IAE to throw IllegalArgumentException instead of NullPointerException

version 3.0.0
=============

- initial release with support for jspecify annotations
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Usage
- add the plugin in `build.gradle`:
```
plugins {
id 'com.dua3.cabe' version '3.0.0'
id 'com.dua3.cabe' version '3.0.1-SNAPSHOT'
}
```

Expand Down
38 changes: 20 additions & 18 deletions Writerside/topics/cabe.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,13 @@ When in doubt, profile your application when compiled using the different settin
This depends on the configuration used. There are four types of checks. The examples assume a non-nullable parameter
#named `p`.

| Check | Equivalent Java code |
|--------------------|-------------------------------------------------------------|
| NO_CHECK | [N/A] |
| STANDARD_ASSERTION | `assert p!=null : "p is null";` |
| ASSERT_ALWAYS | `if (p==null) throw new AssertionError("p is null");` |
| THROW_NPE | `if (p==null) throw new NullPointerException("p is null");` |
| Check | Equivalent Java code |
|--------------------|-----------------------------------------------------------------|
| NO_CHECK | [N/A] |
| STANDARD_ASSERTION | `assert p!=null : "p is null";` |
| ASSERT_ALWAYS | `if (p==null) throw new AssertionError("p is null");` |
| THROW_NPE | `if (p==null) throw new NullPointerException("p is null");` |
| THROW_IAE | `if (p==null) throw new IllegalArgumentException("p is null");` |

## Public vs Private API

Expand Down Expand Up @@ -415,18 +416,19 @@ When using a configuration String, you can use either

Examples:

| Configuration String | Public API | Private API | Return Value |
|------------------------------------------|---------------|---------------|----------------|
| "STANDARD" | THROW_NPE | ASSERT | NO_CHECK |
| "DEVELOPMENT" | ASSERT_ALWAYS | ASSERT_ALWAYS | ASSERT_ALWAYS |
| "NO_CHECKS" | NO_CHECK | NO_CHECK | NO_CHECK |
| "THROW_NPE" | THROW_NPE | THROW_NPE | THROW_NPE |
| "ASSERT" | ASSERT | ASSERT | ASSERT |
| "ASSERT_ALWAYS" | ASSERT_ALWAYS | ASSERT_ALWAYS | ASSERT_ALWAYS |
| "NO_CHECK" | NO_CHECK | NO_CHECK | NO_CHECK |
| "THROW_NPE" | THROW_NPE | THROW_NPE | THROW_NPE |
| "publicApi=THROW_NPE" | THROW_NPE | NO_CHECK | NO_CHECK |
| "publicApi=THROW_NPE:returnValue=ASSERT" | THROW_NPE | NO_CHECK | ASSERT |
| Configuration String | Public API | Private API | Return Value |
|------------------------------------------|---------------|---------------|---------------|
| "STANDARD" | THROW_NPE | ASSERT | NO_CHECK |
| "DEVELOPMENT" | ASSERT_ALWAYS | ASSERT_ALWAYS | ASSERT_ALWAYS |
| "NO_CHECKS" | NO_CHECK | NO_CHECK | NO_CHECK |
| "THROW_NPE" | THROW_NPE | THROW_NPE | THROW_NPE |
| "ASSERT" | ASSERT | ASSERT | ASSERT |
| "ASSERT_ALWAYS" | ASSERT_ALWAYS | ASSERT_ALWAYS | ASSERT_ALWAYS |
| "NO_CHECK" | NO_CHECK | NO_CHECK | NO_CHECK |
| "THROW_NPE" | THROW_NPE | THROW_NPE | THROW_NPE |
| "publicApi=THROW_NPE" | THROW_NPE | NO_CHECK | NO_CHECK |
| "publicApi=THROW_NPE:returnValue=ASSERT" | THROW_NPE | NO_CHECK | ASSERT |
| "publicApi=THROW_IAE:privateApi=ASSERT" | THROW_IAE | ASSERT | NO_CHECK |

You can also use the standard record constructor of <code>Configuration</code>

Expand Down
4 changes: 2 additions & 2 deletions Writerside/v.list
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
<!DOCTYPE vars SYSTEM "https://resources.jetbrains.com/writerside/1.0/vars.dtd">
<vars>
<var name="product" value="Writerside"/>
<var name="PROCESSOR_VERSION" value="3.0.0"/>
<var name="PLUGIN_VERSION" value="3.0.0"/>
<var name="PROCESSOR_VERSION" value="3.0.1-SNAPSHOT"/>
<var name="PLUGIN_VERSION" value="3.0.1-SNAPSHOT"/>
</vars>
4 changes: 2 additions & 2 deletions build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ plugins {
id("com.dorongold.task-tree") version "2.1.1"
}

extra["plugin_version"] = "3.0.0"
extra["processor_version"] = "3.0.0"
extra["plugin_version"] = "3.0.1-SNAPSHOT"
extra["processor_version"] = "3.0.1-SNAPSHOT"

subprojects {
apply(plugin = "java")
Expand Down
124 changes: 59 additions & 65 deletions cabe-processor/src/main/java/com/dua3/cabe/processor/ClassPatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,6 @@ private void instrumentMethod(ClassInfo ci, MethodInfo mi) throws ClassFileProce
&& mi.methodName().equals("equals") && mi.parameters().size() == 1;

LOG.fine(() -> "instrumenting method " + methodName);
boolean hasStandardParameterAssertions = false;
boolean hasOtherParameterChecks = false;
boolean hasStandardReturnValueAssertions = false;
boolean hasOtherReturnValueChecks = false;
try (Formatter standardParameterAssertions = new Formatter();
Expand All @@ -432,48 +430,24 @@ private void instrumentMethod(ClassInfo ci, MethodInfo mi) throws ClassFileProce
NullnessOperator nullnessOperatorParameter = pi.nullnessOperator();
boolean isNonNull = (nullnessOperatorParameter == NullnessOperator.MINUS_NULL)
|| (!ignoreNonMethodNonNullAnnotation && ci.nullnessOperator().andThen(nullnessOperatorParameter) == NullnessOperator.MINUS_NULL);

if (isNonNull) {
Configuration.Check check = ci.isPublicApi() && mi.isPublic() ? configuration.publicApi() : configuration.privateApi();
if (ci.isRecord() && check== Configuration.Check.ASSERT) { // issue: https://github.com/xzel23/cabe/issues/1
LOG.info("cannot use assert in record " + ci.name() + " using THROW_NPE instead / https://github.com/xzel23/cabe/issues/1");
check = Configuration.Check.THROW_NPE;
}
switch (check) {
case NO_CHECK -> {
// nop
}
case ASSERT -> {
standardParameterAssertions.format(
" if (%1$s==null) { throw new AssertionError((Object) \"%2$s is null\"); }%n",
pi.param(), parameterName
);
hasStandardParameterAssertions = true;
}
case ASSERT_ALWAYS -> {
otherParameterChecks.format(
"if (%1$s==null) { throw new AssertionError((Object) \"%2$s is null\"); }%n",
pi.param(), parameterName
);
hasOtherParameterChecks = true;
}
case THROW_NPE -> {
otherParameterChecks.format(
"if (%1$s==null) { throw new NullPointerException(\"%2$s is null\"); }%n",
pi.param(), parameterName
);
hasOtherParameterChecks = true;
}
}
Configuration.Check check = getCheck(ci, mi);
check.getCodeForNewInstance("\"%2$s is null\"")
.map(createThrowableCode -> " if (%1$s==null) { throw " + createThrowableCode + "; }%n")
.ifPresent(checkCode -> {
if (check == Configuration.Check.ASSERT) {
standardParameterAssertions.format(checkCode, pi.param(), parameterName);
} else {
otherParameterChecks.format(checkCode, pi.param(), parameterName);
}
});
LOG.fine(() -> "adding null check for parameter " + parameterName + " in " + ci.name());
}
}

// modify class by injecting parameter checks
String codeParamChecks = (hasStandardParameterAssertions
? "if (%1$s) {%n%2$s}%n".formatted(getAssertionEnabledExpression(ci), standardParameterAssertions)
: "")
+ (hasOtherParameterChecks ? otherParameterChecks : "");

String codeParamChecks = getCheckCode(ci, standardParameterAssertions.toString(), otherParameterChecks.toString());
if (!codeParamChecks.isEmpty()) {
LOG.fine(() -> "injecting code into: " + methodName + "\n" + codeParamChecks.indent(2).stripTrailing());
ctBehavior.insertBefore(codeParamChecks);
Expand All @@ -489,37 +463,20 @@ private void instrumentMethod(ClassInfo ci, MethodInfo mi) throws ClassFileProce
if (isNonNullRV) {
Configuration.Check check = configuration.checkReturn();

switch (check) {
case NO_CHECK -> {
// nop
}
case ASSERT -> {
standardReturnValueAssertions.format(
" if ($_==null) { throw new AssertionError((Object) \"invalid null return value\"); }%n"
);
hasStandardReturnValueAssertions = true;
}
case ASSERT_ALWAYS -> {
otherReturnValueChecks.format(
" if ($_==null) { throw new AssertionError((Object) \"invalid null return value\"); }%n"
);
hasOtherReturnValueChecks = true;
}
case THROW_NPE -> {
otherReturnValueChecks.format(
"if ($_==null) { throw new NullPointerException(\"invalid null return value\"); }%n"
);
hasOtherReturnValueChecks = true;
}
}
check.getCodeForNewInstance("\"invalid null return value\"")
.map(createThrowableCode -> " if ($_==null) { throw " + createThrowableCode + "; }%n")
.ifPresent(checkCode -> {
if (check == Configuration.Check.ASSERT) {
standardReturnValueAssertions.format(checkCode);
} else {
otherReturnValueChecks.format(checkCode);
}
});
LOG.fine(() -> "adding null check for return value in " + ci.name());
}

// modify class by injecting return value checks
String codeReturnValueChecks = (hasStandardReturnValueAssertions
? "if (%1$s) {%n%2$s}%n".formatted(getAssertionEnabledExpression(ci), standardReturnValueAssertions)
: "")
+ (hasOtherReturnValueChecks ? otherReturnValueChecks : "");
String codeReturnValueChecks = getCheckCode(ci, standardReturnValueAssertions.toString(), otherReturnValueChecks.toString());

if (!codeReturnValueChecks.isEmpty()) {
LOG.fine(() -> "injecting code into: " + methodName + "\n" + codeReturnValueChecks.indent(2).stripTrailing());
Expand All @@ -535,6 +492,43 @@ private void instrumentMethod(ClassInfo ci, MethodInfo mi) throws ClassFileProce
}
}

/**
* Constructs a string of code that includes checks for parameter or return value assertions.
*
* @param ci the ClassInfo object representing the class being processed
* @param standardAssertionsCode the code for standard assertions to be included in the check
* @param otherAssertionsCode additional assertion code to be included in the check
* @return a String representing the complete check code
* @throws CannotCompileException if the code compilation fails
* @throws NotFoundException if a required class or method is not found
*/
private String getCheckCode(ClassInfo ci, String standardAssertionsCode, String otherAssertionsCode) throws CannotCompileException, NotFoundException {
return (!standardAssertionsCode.isEmpty()
? "if (%1$s) {%n%2$s}%n".formatted(getAssertionEnabledExpression(ci), standardAssertionsCode)
: "")
+ otherAssertionsCode;
}

/**
* Determines the appropriate check configuration for the given class and method.
*
* <p>For record constructors, standard assertions cannot be injected for technical reasons, so in that case
* {@code Check.THROW_NPE} will be returned instead of {@code Check.ASSERT}.
*
* @param ci the ClassInfo object representing the class being processed
* @param mi the MethodInfo object representing the method being processed
* @return the check configuration to be used, which could be a public API check,
* private API check, or adjusted if the class is a record
*/
private Configuration.Check getCheck(ClassInfo ci, MethodInfo mi) {
Configuration.Check check = ci.isPublicApi() && mi.isPublic() ? configuration.publicApi() : configuration.privateApi();
if (ci.isRecord() && check == Configuration.Check.ASSERT) { // issue: https://github.com/xzel23/cabe/issues/1
LOG.info("cannot use assert in record " + ci.name() + " using THROW_NPE instead / https://github.com/xzel23/cabe/issues/1");
check = Configuration.Check.THROW_NPE;
}
return check;
}

/**
* Retrieves a CtBehavior object representing a method or constructor from the given CtClass that matches the provided MethodInfo.
*
Expand Down
41 changes: 35 additions & 6 deletions cabe-processor/src/main/java/com/dua3/cabe/processor/Configuration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,48 @@ public enum Check {
/**
* Do not generate Checks.
*/
NO_CHECK,
NO_CHECK(null, null),

/**
* Generate standard assertions that can be controlled at runtime (i.e., g {@code -ea} and {@code -da} JVM flags).
*
* <p>If assertions are enabled, violations will result in throwing an {@link AssertionError}.
*/

ASSERT("new AssertionError((Object) ", ")"),

/**
* Throw a {@link NullPointerException} if parameter is null.
*/
THROW_NPE("new NullPointerException(",")"),

/**
* Throw an {@link AssertionError} if parameter is null, regardless of the assertion setting.
*/
ASSERT,
ASSERT_ALWAYS("new AssertionError((Object) ", ")"),

/**
* Throw a NullPointerException if parameter is null.
* Throw an {@link IllegalArgumentException} if parameter is null.
*/
THROW_NPE,
THROW_IAE("new IllegalArgumentException(",")");

private final String prefix;
private final String suffix;

Check(String prefix, String suffix) {
this.prefix = prefix;
this.suffix = suffix;
}

/**
* Throw an AssertionError if parameter is null, regardless of the assertion setting.
* Generate the code fragment that creates a new instance of this Check instance's throwable class.
* @param message the text to use as the message parameter of the throwable constructor.
*
* @return the code to create an instance of this Check instance's throwable
*/
ASSERT_ALWAYS
public Optional<String> getCodeForNewInstance(String message) {
return Optional.ofNullable(prefix).map(pre -> pre + message + suffix);
}
}

}
Loading

0 comments on commit e8ab780

Please sign in to comment.