Add string extensions quote and reverse

[pkwarren]

Add strings.quote and reverse extensions to match Go implementations.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[pkwarren]

I wasn't sure if these were left out of the string extensions intentionally, but these were two that I saw implemented in cel-go but weren't in cel-java. The third one format is much more complex so I didn't tackle that one.

[l46kok]

I wasn't sure if these were left out of the string extensions intentionally, but these were two that I saw implemented in cel-go but weren't in cel-java. The third one format is much more complex so I didn't tackle that one.

Thanks for the contribution. This was just work we hadn't been able to get to, and should definitely be added in CEL-Java.

I'll review more thoroughly soon but in the meanwhile -- would you mind enabling the conformance tests for strings.quote by removing these lines:

https://github.com/google/cel-java/blob/main/conformance/src/test/java/dev/cel/conformance/BUILD.bazel#L123
https://github.com/google/cel-java/blob/main/conformance/src/test/java/dev/cel/conformance/BUILD.bazel#L152

[pkwarren]

Enabled strings.quote in conformance tests in 6207811.

[l46kok]

Please add the newly added function to the documentation: https://github.com/google/cel-java/blob/main/extensions/src/main/java/dev/cel/extensions/README.md?plain=1#L388

You should be able to copy the relevant sections from go's README: https://github.com/google/cel-go/blob/master/ext/README.md

[l46kok]

Could you add a test for the replacement behavior here? Something similar to cel-go's test here should work: https://github.com/google/cel-go/blob/7114ed27a63255f33c689fbff0ee8a08298f70ab/ext/strings_test.go#L623

We should also add tests for low/high surrogate boundaries:

// Exact boundary at i + 1 >= s.length()
end with high surrogate \uD83D -> "end with high surrogate \uFFFD"

// \uD83D is a high surrogate, followed immediately by 'A' instead of a low surrogate
bad pair \uD83DA -> "bad pair \uFFFDA"

[l46kok]

For readability, would you mind extracting this into a separate helper method? The last line checking for unpaired high surrogate is especially worth pulling out into a named variable or adding a comment around it:

private static boolean isMalformedUtf16(String s, int index, int codePoint) {
    char currentChar = s.charAt(index);
    if (!Character.isValidCodePoint(codePoint)) {
        return true;
    }
    if (Character.isLowSurrogate(currentChar)) {
        return true;
    }
    // Check for unpaired high surrogate
    return Character.isHighSurrogate(currentChar) 
        && (index + 1 >= s.length() || !Character.isLowSurrogate(s.charAt(index + 1)));
}

[l46kok]

Could we also port the test over for invisible formatting characters?

https://github.com/google/cel-go/blob/7114ed27a63255f33c689fbff0ee8a08298f70ab/ext/strings_test.go#L111

[l46kok]

Boundary cases for backslashes appearing at the very beginning or the end are also worth adding:

https://github.com/google/cel-go/blob/7114ed27a63255f33c689fbff0ee8a08298f70ab/ext/strings_test.go#L128-L129

[l46kok]

Could you add a test for single quote case as well:

https://github.com/google/cel-go/blob/7114ed27a63255f33c689fbff0ee8a08298f70ab/ext/strings_test.go#L132