google / google/error-prone

Check MessageFormat patterns and arguments, including in methods that delegate to MessageFormat

Open
#6,113 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

### Problem

In pgjdbc, every user-visible error message goes through `org.postgresql.util.GT.tr(String message, Object... args)`, which looks the message up in a resource bundle and always passes it to `java.text.MessageFormat.format(message, args)` ([GT.java](https://github.com/pgjdbc/pgjdbc/blob/master/pgjdbc/src/main/java/org/postgresql/util/GT.java)). The driver has 588 `GT.tr` calls. I want a compile-time error when a pattern and its arguments disagree, and today nothing in the build reports that.

Two kinds of mistakes reach users. A single apostrophe starts a quoted section in `MessageFormat`, so everything after it, placeholders included, is printed literally. A placeholder index with no matching argument is also printed literally. For example, pgjdbc has this call, reached through a wrapper that forwards its `message` and `Object... values` to `GT.tr`:

```java
throwExceptionAboutParsingError(
"Received MaxResultBuffer parameter can't be parsed. Value received to parse: {0}",
value);
```

With the JDK's `MessageFormat` (OpenJDK 21.0.9):

```text
MessageFormat.format("Received MaxResultBuffer parameter can't be parsed. Value received to parse: {0}", "10Q")
-> Received MaxResultBuffer parameter cant be parsed. Value received to parse: {0}
MessageFormat.format("{0} and {1}", "only-one")
-> only-one and {1}
```

The user never sees the value they passed. pgjdbc has four more calls with an unescaped apostrophe: one more in the same parser, which loses the value the same way, and three where only the apostrophe disappears from the message.

`@FormatMethod` with `FormatStringAnnotation` does not fit: its Javadoc defines it for printf-style format strings, and `MessageFormat` uses `{0}` placeholders and apostrophe quoting instead.

### Expected behavior

For a call to `MessageFormat.format(String, Object...)`, and to a method marked as delegating to it, with a compile-time constant pattern, Error Prone reports:

1. a pattern that `new MessageFormat(pattern)` rejects with `IllegalArgumentException`;
2. a placeholder index that has no argument (`"{0} and {1}"` with one argument);
3. an argument that no placeholder uses;
4. an apostrophe that quotes a placeholder (`"can't ... {0}"`), which is the `MaxResultBuffer` case above.

Acceptance: compiling the call above reports case 4 on the pattern argument, and compiling the corrected `"can''t ... {0}"` reports nothing.

### What I would like the maintainers to decide

The requirement is the check. How a project marks its own delegating methods is yours to choose, and it affects whether pgjdbc has to add a dependency. Three possible shapes:

- a new annotation in `error_prone_annotations`, such as `@MessageFormatMethod`, next to `@FormatMethod`;
- a flag that lists delegating methods, such as `-XepOpt:MessageFormat:Methods=org.postgresql.util.GT#tr`;
- a flag that lists annotation names to treat as the marker, which would also answer #1440 for `@FormatMethod`.

Is any of these acceptable for Error Prone? Are custom marker annotations, matched by name or configured through a flag, something the project wants to support?

### Out of scope

Choice and date/number sub-formats beyond checking that the pattern parses, translated patterns loaded from resource bundles at runtime, and any change to the printf checks.

### Prior discussion

- #231 asked for a MessageFormat check in 2014 and was closed as fixed by `MisusedFormattingLogger`. That check matched only `com.google.common.logging.FormattingLogger` and `com.google.gdata.util.common.logging.FormattingLogger`, and commit 9b4b74348d moved it to internal-only checks in 2015.
- #1440 asks to treat the Checker Framework's `@FormatMethod` as an alias of Error Prone's; it has no response.

I searched issues for `MessageFormat`, `FormatMethod`, `LenientFormatString`, and `custom annotation flag`.

### Alternatives

A custom `BugChecker` plugin inside pgjdbc works for pgjdbc alone. Every project that wraps `MessageFormat` would have to write the same check again.

I am willing to implement this and send a pull request once the shape is agreed.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the existing FormatMethod handling and the MessageFormat cases described in the issue, including delegated methods such as org.postgresql.util.GT#tr. Define how delegating methods are marked or configured, then add checks for invalid patterns, missing or unused arguments, and apostrophes quoting placeholders; verify the listed acceptance examples and corrected pattern.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
devtools
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.