This is an automated email from the ASF dual-hosted git repository. paulk pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push: new 5a3cf7c86f GROOVY-11261: Provide a custom type checker for format strings (additional tests) 5a3cf7c86f is described below commit 5a3cf7c86feacc6f257cc72942e6ef8789920cf4 Author: Paul King <pa...@asert.com.au> AuthorDate: Sat Jan 6 10:38:26 2024 +1000 GROOVY-11261: Provide a custom type checker for format strings (additional tests) --- .../groovy/typecheckers/FormatStringChecker.groovy | 31 +++++++++++---- .../src/spec/doc/typecheckers.adoc | 14 +++++-- .../typecheckers/FormatStringCheckerTest.groovy | 46 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/FormatStringChecker.groovy b/subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/FormatStringChecker.groovy index f7ecb10c38..0e08631280 100644 --- a/subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/FormatStringChecker.groovy +++ b/subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/FormatStringChecker.groovy @@ -147,9 +147,28 @@ class FormatStringChecker extends GroovyTypeCheckingExtensionSupport.TypeCheckin void checkFormatStringTypes(Expression expression, List<Expression> args, Expression target) { int next = 0 + int prevIndex = -1 expression.value.eachMatch(formatSpecifier) { spec -> def (all, argIndex, flags, width, precision, tgroup, conversion) = spec - def arg = args[argIndex ?: next] + def flagList = flags?.toList() + if (argIndex) { + argIndex -= '$' + argIndex = argIndex.toInteger() + } + int indexToUse = argIndex ?: next + if (flagList.contains('<')) { + if (prevIndex == -1) { + addStaticTypeError("MissingFormatArgument: Format specifier '$all'", target) + return + } else { + indexToUse = prevIndex + } + } + if (indexToUse >= args.size()) { + addStaticTypeError("MissingFormatArgument: Format specifier '$all'", target) + return + } + def arg = args[indexToUse] Object type = getWrapper(getType(arg)).typeClass if (tgroup) { if (!'HIklMSLNpzZsQBbhAaCYyjmdeRTrDFc'.contains(conversion)) { @@ -161,13 +180,11 @@ class FormatStringChecker extends GroovyTypeCheckingExtensionSupport.TypeCheckin if (!([Long, Calendar, Date, TemporalAccessor].any { it.isAssignableFrom(type) })) { addStaticTypeError("IllegalFormatConversion: $conversion != $type.name", target) } - def flagList = flags?.toList() checkBadFlags(flagList, conversion, target, '#+ 0,(') if (flagList.contains('-') && !width) { addStaticTypeError("MissingFormatWidth: $all", target) } } else { - def flagList = flags?.toList() def dupFlag = flagList.countBy().find { flag, count -> count > 1 }?.key if (dupFlag) { addStaticTypeError("DuplicateFormatFlags: Flags = '$dupFlag'", target) @@ -224,12 +241,10 @@ class FormatStringChecker extends GroovyTypeCheckingExtensionSupport.TypeCheckin addStaticTypeError("UnknownFormatConversion: Conversion = '${conversion}'", target) } } - if ((argIndex ?: next) > args.size()) { - addStaticTypeError("Bad index", target) - } else { - println 'Inferred type: ' + type + prevIndex = indexToUse + if (!argIndex && !flagList.contains('<')) { + next++ } - next++ // TODO look for < } } diff --git a/subprojects/groovy-typecheckers/src/spec/doc/typecheckers.adoc b/subprojects/groovy-typecheckers/src/spec/doc/typecheckers.adoc index eef34b2649..a43e7be5da 100644 --- a/subprojects/groovy-typecheckers/src/spec/doc/typecheckers.adoc +++ b/subprojects/groovy-typecheckers/src/spec/doc/typecheckers.adoc @@ -40,7 +40,7 @@ As usual, Groovy's compiler customization mechanisms would allow you to simplify application of such checkers, e.g. make it apply globally using a compiler configuration script, as just one example. -== Checking Regex literals +== Checking Regular Expressions Consider the following code, which contains 3 regular expressions: @@ -109,7 +109,7 @@ format string and zero or more arguments. These methods have the following characteristics: -* The first argument must be of type api:java.util.Locale[] or api:java.lang.String[]. +* The first argument must be of type `java.util.Locale` or `java.lang.String`. * If the first argument is of type `Locale`, the second argument must be of type `String`. * The `String` argument is treated as a format string containing zero or more embedded format specifiers as well as plain text. The format specifiers determine how the remaining arguments will be used within the resulting output. * The `FormatStringChecker` ensures that the format string is valid and the remaining arguments are compatible with the embedded format specifiers. @@ -152,7 +152,7 @@ sprintf(Locale l, String format, Object... args) ---- |=== -Here is an example of correctly using some of these methods: +Here is an example of correctly using some of these methods with type checking in place: [source,groovy] ---- @@ -170,6 +170,13 @@ include::../test/FormatStringCheckerTest.groovy[tags=format_method_example,inden You can use the `FormatMethod` annotation provided in the `groovy-typecheckers` module or the https://checkerframework.org/api/org/checkerframework/checker/formatter/qual/FormatMethod.html[similarly named] annotation from the Java-based https://checkerframework.org/[checker framework]. +The format string checker looks for a constant literal +format string. For arguments, it also looks for constant +literals or otherwise makes checks based on inferred type. +When looking for constants, the checker will find inline +literals, local variables with a constant initializer, +and fields with a constant initializer. + === Errors detected Here are some examples of the kinds of errors detected. @@ -226,3 +233,4 @@ The complete list of errors detected include: * unknown format conversions * illegal format conversions * format flags conversion mismatches +* missing arguments diff --git a/subprojects/groovy-typecheckers/src/test/groovy/groovy/typecheckers/FormatStringCheckerTest.groovy b/subprojects/groovy-typecheckers/src/test/groovy/groovy/typecheckers/FormatStringCheckerTest.groovy index f0a83f8eac..51f9d0966e 100644 --- a/subprojects/groovy-typecheckers/src/test/groovy/groovy/typecheckers/FormatStringCheckerTest.groovy +++ b/subprojects/groovy-typecheckers/src/test/groovy/groovy/typecheckers/FormatStringCheckerTest.groovy @@ -24,7 +24,9 @@ import org.junit.BeforeClass import org.junit.Test import static groovy.test.GroovyAssert.assertScript +import static groovy.test.GroovyAssert.isAtLeastJdk import static groovy.test.GroovyAssert.shouldFail +import static org.junit.Assume.assumeTrue final class FormatStringCheckerTest { @@ -255,6 +257,38 @@ final class FormatStringCheckerTest { assert err =~ /IllegalFormatConversion: c != java\.lang\.Boolean/ } + @Test + void testMissingFormatArgumentForConstantArgs() { + def err = shouldFail shell, ''' + String.format('%<s', 'some string') + ''' + assert err =~ /MissingFormatArgument: Format specifier '%<s'/ + err = shouldFail shell, ''' + String.format('%s %s', 'some string') + ''' + assert err =~ "MissingFormatArgument: Format specifier '%s'" + err = shouldFail shell, ''' + String.format('%2$s', 'some string') + ''' + assert err =~ /MissingFormatArgument: Format specifier '%\d[$]s'/ + } + + @Test + void testMissingFormatArgumentForInferredArgs() { + def err = shouldFail shell, ''' + String.format('%<s', 'some string' + '') + ''' + assert err =~ /MissingFormatArgument: Format specifier '%<s'/ + err = shouldFail shell, ''' + String.format('%s %s', 'some string' + '') + ''' + assert err =~ "MissingFormatArgument: Format specifier '%s'" + err = shouldFail shell, ''' + String.format('%2$s', 'some string' + '') + ''' + assert err =~ /MissingFormatArgument: Format specifier '%\d[$]s'/ + } + @Test void testValidFormatStringsConstantArgs() { assertScript shell, ''' @@ -275,8 +309,20 @@ final class FormatStringCheckerTest { assert sprintf('%3s', 'abcde') == 'abcde' assert sprintf('%6s', 'abcde') == ' abcde' assert sprintf('%-6s', 'abcde') == 'abcde ' + assert String.format('%2$s %1$s', 'bar', 'foo') == 'foo bar' + assert String.format('%1$s %1$s', 'bar', 'foo') == 'bar bar' + assert String.format('%s %<s', 'bar', 'foo') == 'bar bar' + assert String.format('%2$s %<s', 'bar', 'foo') == 'foo foo' + assert String.format('%2$s %2$s', 'bar', 'foo') == 'foo foo' } ''' } + @Test + void testValidFormattedCall() { + assumeTrue(isAtLeastJdk('15.0')) + assertScript shell, ''' + assert '%x'.formatted(16) == '10' + ''' + } }