diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index af3247a9ebd2..d18e70223c35 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -100,9 +100,10 @@ iterator.hasnext.which.calls.next.problem.descriptor=Iterator.#ref()#ref is malformed #loc -malformed.format.string.problem.descriptor.too.many.arguments=Too many arguments for format string #ref #loc -malformed.format.string.problem.descriptor.too.few.arguments=Too few arguments for format string #ref #loc -malformed.format.string.problem.descriptor.arguments.do.not.match.type=Format string #ref does not match the type of its arguments #loc +malformed.format.string.problem.descriptor.illegal=Illegal format string specifier ''{0}'' #loc +malformed.format.string.problem.descriptor.too.many.arguments=Too many arguments for format string (found: {0}, expected: {1}) #loc +malformed.format.string.problem.descriptor.too.few.arguments=Too few arguments for format string (found: {0}, expected: {1}) #loc +malformed.format.string.problem.descriptor.arguments.do.not.match.type=Argument type ''{0}'' does not match the type of the format specifier ''{1}'' #loc malformed.regular.expression.display.name=Malformed regular expression malformed.regular.expression.problem.descriptor1=Regular expression #ref is malformed #loc malformed.regular.expression.problem.descriptor2=Regular expression #ref is malformed: {0} #loc diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/FormatDecode.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/FormatDecode.java index 9c027f4e96a7..36dea6861de2 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/FormatDecode.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/FormatDecode.java @@ -15,6 +15,7 @@ */ package com.siyeh.ig.bugs; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.CommonClassNames; import com.intellij.psi.PsiType; import com.intellij.psi.util.InheritanceUtil; @@ -37,75 +38,169 @@ class FormatDecode { private static final Validator ALL_VALIDATOR = new AllValidator(); - private static final Validator DATE_VALIDATOR = new DateValidator(); + private static final int LEFT_JUSTIFY = 1; // '-' + private static final int ALTERNATE = 2; // '#' + private static final int PLUS = 4; // '+' + private static final int LEADING_SPACE = 8; // ' ' + private static final int ZERO_PAD = 16; // '0' + private static final int GROUP = 32; // ',' + private static final int PARENTHESES = 64; // '(' + private static final int PREVIOUS = 128; // '<' - private static final Validator CHAR_VALIDATOR = new CharValidator(); - - private static final Validator INT_VALIDATOR = new IntValidator(); - - private static final Validator FLOAT_VALIDATOR = new FloatValidator(); + private static int flag(char c) { + switch (c) { + case '-': return LEFT_JUSTIFY; + case '#': return ALTERNATE; + case '+': return PLUS; + case ' ': return LEADING_SPACE; + case '0': return ZERO_PAD; + case ',': return GROUP; + case '(': return PARENTHESES; + case '<': return PREVIOUS; + default: throw new IllegalFormatException(); + } + } public static Validator[] decode(String formatString, int argumentCount) { final ArrayList parameters = new ArrayList(); final Matcher matcher = fsPattern.matcher(formatString); + boolean previousAllowed = false; int implicit = 0; int pos = 0; - for (int i = 0; matcher.find(i); i = matcher.end()) { + int i = 0; + while (matcher.find(i)) { + final int start = matcher.start(); + if (start != i) { + checkText(formatString.substring(i, start)); + } + i = matcher.end(); + final String specifier = matcher.group(); final String posSpec = matcher.group(1); final String flags = matcher.group(2); + final String width = matcher.group(3); + final String precision = matcher.group(4); final String dateSpec = matcher.group(5); final String spec = matcher.group(6); + int flagBits = 0; + for (int j = 0; j < flags.length(); j++) { + final int bit = flag(flags.charAt(j)); + if ((flagBits | bit) == flagBits) throw new IllegalFormatException(specifier); // duplicate flag + flagBits |= bit; + } + if (isAllBitsSet(flagBits, LEADING_SPACE | PLUS) || isAllBitsSet(flagBits, LEFT_JUSTIFY | ZERO_PAD)) { + // illegal flag combination + throw new IllegalFormatException(specifier); + } + // check this first because it should not affect "implicit" - if ("n".equals(spec) || "%".equals(spec)) { + if ("n".equals(spec)) { + // no flags allowed + if (flagBits != 0 || !StringUtil.isEmpty(width) || !StringUtil.isEmpty(precision)) throw new IllegalFormatException(specifier); + continue; + } + else if ("%".equals(spec)) { + if (isAnyBitSet(flagBits, ~LEFT_JUSTIFY) || !StringUtil.isEmpty(precision)) throw new IllegalFormatException(specifier); continue; } if (posSpec != null) { + if (isAnyBitSet(flagBits, PREVIOUS)) throw new IllegalFormatException(specifier); final String num = posSpec.substring(0, posSpec.length() - 1); pos = Integer.parseInt(num) - 1; + previousAllowed = true; } - else if (flags == null || flags.indexOf('<') < 0) { + else if (isAnyBitSet(flagBits, PREVIOUS)) { + // reuse last pos + if (!previousAllowed) throw new IllegalFormatException(specifier); + } + else { + previousAllowed = true; pos = implicit++; } - // else if the flag has "<" reuse the last pos final Validator allowed; if (dateSpec != null) { // a t or T - allowed = DATE_VALIDATOR; + if (isAnyBitSet(flagBits, ~LEFT_JUSTIFY) || !StringUtil.isEmpty(precision)) throw new IllegalFormatException(specifier); + allowed = new DateValidator(specifier); } else { - switch (Character.toLowerCase(spec.charAt(0))) { + switch (spec.charAt(0)) { case 'b': + case 'B': case 'h': + case 'H': + if (isAnyBitSet(flagBits, ~LEFT_JUSTIFY)) throw new IllegalFormatException(specifier); + allowed = ALL_VALIDATOR; + break; case 's': + case 'S': + if (isAnyBitSet(flagBits, ~(LEFT_JUSTIFY | ALTERNATE))) throw new IllegalFormatException(specifier); allowed = ALL_VALIDATOR; break; case 'c': - allowed = CHAR_VALIDATOR; + case 'C': + if (isAnyBitSet(flagBits, ~LEFT_JUSTIFY) || !StringUtil.isEmpty(precision)) throw new IllegalFormatException(specifier); + allowed = new CharValidator(specifier); break; case 'd': + if (isAnyBitSet(flagBits, ALTERNATE)) throw new IllegalFormatException(specifier); + allowed = new IntValidator(specifier); + break; case 'o': case 'x': - allowed = INT_VALIDATOR; + case 'X': + if (isAnyBitSet(flagBits, PLUS | LEADING_SPACE | GROUP) || !StringUtil.isEmpty(precision)) { + throw new IllegalFormatException(specifier); + } + allowed = new IntValidator(specifier); + break; + case 'a': + case 'A': + if (isAnyBitSet(flagBits, PARENTHESES | GROUP)) throw new IllegalFormatException(specifier); + allowed = new FloatValidator(specifier); break; case 'e': - case 'f': + case 'E': + if (isAnyBitSet(flagBits, GROUP)) throw new IllegalFormatException(specifier); + allowed = new FloatValidator(specifier); + break; case 'g': - case 'a': - allowed = FLOAT_VALIDATOR; + case 'G': + if (isAnyBitSet(flagBits, ALTERNATE)) throw new IllegalFormatException(specifier); + allowed = new FloatValidator(specifier); + break; + case 'f': + allowed = new FloatValidator(specifier); break; default: - throw new UnknownFormatException(matcher.group()); + throw new IllegalFormatException(specifier); } } storeValidator(allowed, pos, parameters, argumentCount); } + if (i < formatString.length() - 1) { + checkText(formatString.substring(i)); + } return parameters.toArray(new Validator[parameters.size()]); } + private static boolean isAnyBitSet(int value, int mask) { + return (value & mask) != 0; + } + + private static boolean isAllBitsSet(int value, int mask) { + return (value & mask) == mask; + } + + private static void checkText(String s) { + if (s.indexOf('%') != -1) { + throw new IllegalFormatException(); + } + } + private static void storeValidator(Validator validator, int pos, ArrayList parameters, int argumentCount) { @@ -118,7 +213,7 @@ class FormatDecode { ((MultiValidator)existing).addValidator(validator); } else if (existing != validator) { - final MultiValidator multiValidator = new MultiValidator(); + final MultiValidator multiValidator = new MultiValidator(existing.getSpecifier()); multiValidator.addValidator(existing); multiValidator.addValidator(validator); parameters.set(pos, multiValidator); @@ -132,14 +227,20 @@ class FormatDecode { } } - public static class UnknownFormatException extends RuntimeException { + public static class IllegalFormatException extends RuntimeException { - public UnknownFormatException(String message) { + public IllegalFormatException(String message) { super(message); } + + public IllegalFormatException() {} } - private static class AllValidator implements Validator { + private static class AllValidator extends Validator { + + public AllValidator() { + super(""); + } @Override public boolean valid(PsiType type) { @@ -147,7 +248,11 @@ class FormatDecode { } } - private static class DateValidator implements Validator { + private static class DateValidator extends Validator { + + public DateValidator(String specifier) { + super(specifier); + } @Override public boolean valid(PsiType type) { @@ -159,7 +264,11 @@ class FormatDecode { } } - private static class CharValidator implements Validator { + private static class CharValidator extends Validator { + + public CharValidator(String specifier) { + super(specifier); + } @Override public boolean valid(PsiType type) { @@ -174,7 +283,11 @@ class FormatDecode { } } - private static class IntValidator implements Validator { + private static class IntValidator extends Validator { + + public IntValidator(String specifier) { + super(specifier); + } @Override public boolean valid(PsiType type) { @@ -191,7 +304,11 @@ class FormatDecode { } } - private static class FloatValidator implements Validator { + private static class FloatValidator extends Validator { + + public FloatValidator(String specifier) { + super(specifier); + } @Override public boolean valid(PsiType type) { @@ -204,9 +321,13 @@ class FormatDecode { } } - private static class MultiValidator implements Validator { + private static class MultiValidator extends Validator { private final Set validators = new HashSet(3); + public MultiValidator(String specifier) { + super(specifier); + } + @Override public boolean valid(PsiType type) { for (Validator validator : validators) { @@ -221,4 +342,19 @@ class FormatDecode { validators.add(validator); } } + + abstract static class Validator { + + private final String mySpecifier; + + public Validator(String specifier) { + mySpecifier = specifier; + } + + public abstract boolean valid(PsiType type); + + public String getSpecifier() { + return mySpecifier; + } + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MalformedFormatStringInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MalformedFormatStringInspectionBase.java index a08902b77216..9e700f827a3a 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MalformedFormatStringInspectionBase.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MalformedFormatStringInspectionBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2014 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2015 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -78,17 +78,27 @@ public class MalformedFormatStringInspectionBase extends BaseInspection { public String buildErrorString(Object... infos) { final Object value = infos[0]; if (value instanceof Exception) { + final Exception exception = (Exception)value; + final String message = exception.getMessage(); + if (message != null) { + return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.illegal", message); + } return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.malformed"); } - final Validator[] validators = (Validator[])value; + final FormatDecode.Validator[] validators = (FormatDecode.Validator[])value; final int argumentCount = ((Integer)infos[1]).intValue(); if (validators.length < argumentCount) { - return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.too.many.arguments"); + return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.too.many.arguments", + argumentCount, validators.length); } if (validators.length > argumentCount) { - return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.too.few.arguments"); + return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.too.few.arguments", + argumentCount, validators.length); } - return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.arguments.do.not.match.type"); + final PsiType argumentType = (PsiType)infos[2]; + final FormatDecode.Validator validator = (FormatDecode.Validator)infos[3]; + return InspectionGadgetsBundle.message("malformed.format.string.problem.descriptor.arguments.do.not.match.type", + argumentType.getPresentableText(), validator.getSpecifier()); } @Override @@ -142,7 +152,7 @@ public class MalformedFormatStringInspectionBase extends BaseInspection { return; } final int argumentCount = arguments.length - (formatArgumentIndex + 1); - final Validator[] validators; + final FormatDecode.Validator[] validators; try { validators = FormatDecode.decode(value, argumentCount); } @@ -158,17 +168,18 @@ public class MalformedFormatStringInspectionBase extends BaseInspection { return; } } - registerError(formatArgument, validators, Integer.valueOf(argumentCount)); + registerMethodCallError(expression, validators, Integer.valueOf(argumentCount)); return; } for (int i = 0; i < validators.length; i++) { - final Validator validator = validators[i]; - final PsiType argumentType = arguments[i + formatArgumentIndex + 1].getType(); + final FormatDecode.Validator validator = validators[i]; + final PsiExpression argument = arguments[i + formatArgumentIndex + 1]; + final PsiType argumentType = argument.getType(); if (argumentType == null) { continue; } if (validator != null && !validator.valid(argumentType)) { - registerError(formatArgument, validators, Integer.valueOf(argumentCount)); + registerError(argument, validators, Integer.valueOf(argumentCount), argumentType, validator); return; } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/Validator.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/Validator.java deleted file mode 100644 index bdf4e3156ab0..000000000000 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/Validator.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2003-2014 Dave Griffith, Bas Leijdekkers - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.siyeh.ig.bugs; - -import com.intellij.psi.PsiType; - -interface Validator { - - boolean valid(PsiType type); -} diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java index 0302496818f9..773ab2bccce2 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/malformed_format_string/MalformedFormatString.java @@ -7,30 +7,30 @@ public class MalformedFormatString { public void foo() { - String.format("%", 3.0); - System.out.printf("%", 3.0); - System.out.printf("%q", 3.0); - System.out.printf("%d", 3.0); - System.out.printf(new Locale(""),"%d%s", 3.0, "foo"); + String.format("%", 3.0); + System.out.printf("%", 3.0); + System.out.printf("%q", 3.0); + System.out.printf("%d", 3.0); + System.out.printf(new Locale(""),"%d%s", 3.0, "foo"); } public static void main(String[] args) { String local = "hmm"; String good = String.format("%s %s", 1, 2); // this is valid according to the inspector (correct) - String warn = String.format("%s %s", 1); // this is invalid according to the inspector (correct) + String warn = String.format("%s %s", 1); // this is invalid according to the inspector (correct) String invalid = String.format("%s %s" + local, 1); // this is valid according to the inspector (INCORRECT!) - String interesting = String.format("%s %s" + "hmm", 1); // this is invalid according to the inspector (correct) + String interesting = String.format("%s %s" + "hmm", 1); // this is invalid according to the inspector (correct) String intAsChar = String.format("symbol '%1$c' (numeric value %1$d)", 60); // integer->char conversion is ok (correct) } public void outOfMemory() { - String.format("%2147483640$s", "s"); + String.format("%2147483640$s", "s"); } public void optionalSettings() { SomeOtherLogger logger = new SomeOtherLogger(); - logger.d("%s %s", 1); // this is invalid according to the inspector (correct) + logger.d("%s %s", 1); // this is invalid according to the inspector (correct) } public class SomeOtherLogger { @@ -40,7 +40,7 @@ public class MalformedFormatString { } void shouldWarn() { - String.format("%1$c %1$d", 10L); + String.format("%1$c %1$d", 10L); } void shouldNotWarn() { @@ -51,4 +51,43 @@ public class MalformedFormatString { String timestamp(Timestamp ts) { return String.format("%tF %tT", ts, ts); } + + void badStrings() { + // bad format specifier + String.format("%) %n"); + + // flags on newline not allowed + String.format("%(n"); + + // unknown conversion + String.format("%D", 1); + + // duplicate leading space flag + String.format("% d", 1); + + // illegal alternate flag + String.format("%#B", true); + + // illegal flag combination + String.format("% +d", 1); + + // illegal flag on date/time + String.format("%+T", new Timestamp(0)); + + // previous flag without previous + String.format("%, 1); + + // illegal flag + String.format("%(s", 1); + + // unknown format conversions + String.format("%F", 1.0); + String.format("%D", 1); + String.format("%O", 1); + } + + void goodStrings() { + String.format("%-B", true); // left justify flag + String.format("%,d", 34567890); + } }