Skip to content

Commit baf72d1

Browse files
committed
handle null getLineMap() correctly
1 parent 5e66d8d commit baf72d1

File tree

1 file changed

+79
-80
lines changed

1 file changed

+79
-80
lines changed

mug-errorprone/src/main/java/com/google/mu/errorprone/StringFormatArgsCheck.java

Lines changed: 79 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,6 @@ private void checkTemplateFormatArgs(
241241
List<String> argSources,
242242
VisitorState state)
243243
throws ErrorReport {
244-
if (argSources.isEmpty()) {
245-
return;
246-
}
247244
int templateStringIndex =
248245
BiStream.zip(indexesFrom(0), params.stream())
249246
.filterValues(
@@ -278,86 +275,88 @@ private void checkFormatArgs(
278275
for (ExpressionTree arg : args) {
279276
checkArgFormattability(arg, state);
280277
}
281-
ImmutableList<String> normalizedArgTexts = normalizeForComparison(argSources);
282-
LineMap lineMap = state.getPath().getCompilationUnit().getLineMap();
283-
for (int i = 0; i < placeholders.size(); i++) {
284-
Placeholder placeholder = placeholders.get(i);
285-
NodeCheck onPlaceholder = checkingOn(() -> placeholder.sourcePosition(formatExpression, state));
286-
onPlaceholder.require(
287-
args.size() > i, "No value is provided for placeholder {%s}", placeholder.name());
288-
String normalizedPlacehoderName = normalizeForComparison(placeholder.name());
289-
ExpressionTree arg = args.get(i);
290-
if (!normalizedArgTexts.get(i).contains(normalizedPlacehoderName)) {
291-
// arg doesn't match placeholder
292-
boolean trust =
293-
formatStringIsInlined
294-
&& args.size() <= 3
295-
&& arg instanceof LiteralTree
296-
&& (args.size() <= 1
297-
|| normalizedArgTexts.stream() // out-of-order is suspicious
298-
.noneMatch(txt -> txt.contains(normalizedPlacehoderName)));
299-
checkingOn(arg)
300-
.require(
301-
trust && !SourceUtils.hasArgComment(argSources.get(i)),
302-
"String format placeholder {%s} at line %s (defined as in \"%s\") should appear in the format"
303-
+ " argument: %s. Consider the following to address this error:\n"
304-
+ " 1. Ensure the argument isn't passed in out of order.\n"
305-
+ " 2. If the argument does correspond to the placeholder positionally, rename"
306-
+ " either the placeholder {%s} or local variable used in the argument, if any,"
307-
+ " to make the argument expression include the placeholder name word-by-word"
308-
+ " (case insensitive)\n"
309-
+ " 3. If you can't make them organically match, as the last resort, add a"
310-
+ " comment like /* %s */ before the argument. You only need to add the comment"
311-
+ " for non-matching placeholders. Don't add redundant comments for the"
312-
+ " placeholders that already match.",
313-
placeholder.name(),
314-
lineMap.getLineNumber(
315-
placeholder
316-
.sourcePosition(formatExpression, state)
317-
.getPreferredPosition()),
318-
placeholder,
319-
arg,
320-
placeholder.name(),
321-
placeholder.name());
322-
}
323-
if (placeholder.hasConditionalOperator()) {
324-
Type argType = ASTHelpers.getType(arg);
325-
ImmutableSet<String> references = placeholder.optionalParametersFromOperatorRhs();
326-
if (ASTHelpers.isSameType(argType, state.getSymtab().booleanType, state)
327-
|| BOOLEAN_TYPE.isSameType(argType, state)) {
328-
onPlaceholder.require(
329-
references.isEmpty(),
330-
"guard placeholder {%s ->} maps to boolean expression <%s> at line %s. The optional"
331-
+ " placeholder references %s to the right of the `->` operator should only be"
332-
+ " used for an optional placeholder.",
333-
placeholder.name(),
334-
arg,
335-
lineMap.getLineNumber(ASTHelpers.getStartPosition(arg)),
336-
references);
337-
} else if (OPTIONAL_TYPE.isSameType(argType, state) || COLLECTION_TYPE.isSupertypeOf(argType, state)) {
338-
onPlaceholder
339-
.require(
340-
placeholder.hasOptionalParameter(),
341-
"optional parameter {%s->} must be an identifier followed by a '?'",
342-
placeholder.name())
278+
if (argSources.size() > 0) { // skip if we failed to get the arg sources or if there are 0 args.
279+
ImmutableList<String> normalizedArgTexts = normalizeForComparison(argSources);
280+
LineMap lineMap = state.getPath().getCompilationUnit().getLineMap();
281+
for (int i = 0; i < placeholders.size(); i++) {
282+
Placeholder placeholder = placeholders.get(i);
283+
NodeCheck onPlaceholder = checkingOn(() -> placeholder.sourcePosition(formatExpression, state));
284+
onPlaceholder.require(
285+
args.size() > i, "No value is provided for placeholder {%s}", placeholder.name());
286+
String normalizedPlacehoderName = normalizeForComparison(placeholder.name());
287+
ExpressionTree arg = args.get(i);
288+
if (!normalizedArgTexts.get(i).contains(normalizedPlacehoderName)) {
289+
// arg doesn't match placeholder
290+
boolean trust =
291+
formatStringIsInlined
292+
&& args.size() <= 3
293+
&& arg instanceof LiteralTree
294+
&& (args.size() <= 1
295+
|| normalizedArgTexts.stream() // out-of-order is suspicious
296+
.noneMatch(txt -> txt.contains(normalizedPlacehoderName)));
297+
checkingOn(arg)
343298
.require(
344-
!references.isEmpty(),
345-
"optional parameter %s must be referenced at least once to the right of {%s->}",
299+
trust && !SourceUtils.hasArgComment(argSources.get(i)),
300+
"String format placeholder {%s} at line %s (defined as in \"%s\") should appear in the format"
301+
+ " argument: %s. Consider the following to address this error:\n"
302+
+ " 1. Ensure the argument isn't passed in out of order.\n"
303+
+ " 2. If the argument does correspond to the placeholder positionally, rename"
304+
+ " either the placeholder {%s} or local variable used in the argument, if any,"
305+
+ " to make the argument expression include the placeholder name word-by-word"
306+
+ " (case insensitive)\n"
307+
+ " 3. If you can't make them organically match, as the last resort, add a"
308+
+ " comment like /* %s */ before the argument. You only need to add the comment"
309+
+ " for non-matching placeholders. Don't add redundant comments for the"
310+
+ " placeholders that already match.",
346311
placeholder.name(),
347-
placeholder.name())
348-
.require(
349-
references.equals(ImmutableSet.of(placeholder.cleanName())),
350-
"unexpected optional parameters to the right of {%s->}: %s",
312+
lineMap.getLineNumber(
313+
placeholder
314+
.sourcePosition(formatExpression, state)
315+
.getPreferredPosition()),
316+
placeholder,
317+
arg,
351318
placeholder.name(),
352-
Sets.difference(references, ImmutableSet.of(placeholder.name())));
353-
} else {
354-
throw onPlaceholder.report(
355-
"guard placeholder {%s ->} is expected to be boolean, Optional or Collection, whereas"
356-
+ " argument <%s> at line %s is of type %s",
357-
placeholder.name(),
358-
arg,
359-
lineMap.getLineNumber(ASTHelpers.getStartPosition(arg)),
360-
argType);
319+
placeholder.name());
320+
}
321+
if (placeholder.hasConditionalOperator()) {
322+
Type argType = ASTHelpers.getType(arg);
323+
ImmutableSet<String> references = placeholder.optionalParametersFromOperatorRhs();
324+
if (ASTHelpers.isSameType(argType, state.getSymtab().booleanType, state)
325+
|| BOOLEAN_TYPE.isSameType(argType, state)) {
326+
onPlaceholder.require(
327+
references.isEmpty(),
328+
"guard placeholder {%s ->} maps to boolean expression <%s> at line %s. The optional"
329+
+ " placeholder references %s to the right of the `->` operator should only be"
330+
+ " used for an optional placeholder.",
331+
placeholder.name(),
332+
arg,
333+
lineMap.getLineNumber(ASTHelpers.getStartPosition(arg)),
334+
references);
335+
} else if (OPTIONAL_TYPE.isSameType(argType, state) || COLLECTION_TYPE.isSupertypeOf(argType, state)) {
336+
onPlaceholder
337+
.require(
338+
placeholder.hasOptionalParameter(),
339+
"optional parameter {%s->} must be an identifier followed by a '?'",
340+
placeholder.name())
341+
.require(
342+
!references.isEmpty(),
343+
"optional parameter %s must be referenced at least once to the right of {%s->}",
344+
placeholder.name(),
345+
placeholder.name())
346+
.require(
347+
references.equals(ImmutableSet.of(placeholder.cleanName())),
348+
"unexpected optional parameters to the right of {%s->}: %s",
349+
placeholder.name(),
350+
Sets.difference(references, ImmutableSet.of(placeholder.name())));
351+
} else {
352+
throw onPlaceholder.report(
353+
"guard placeholder {%s ->} is expected to be boolean, Optional or Collection, whereas"
354+
+ " argument <%s> at line %s is of type %s",
355+
placeholder.name(),
356+
arg,
357+
lineMap.getLineNumber(ASTHelpers.getStartPosition(arg)),
358+
argType);
359+
}
361360
}
362361
}
363362
}

0 commit comments

Comments
 (0)