diff options
10 files changed, 360 insertions, 10 deletions
diff --git a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java index c43a8ed..e4975ae 100644 --- a/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java +++ b/lint/libs/lint_api/src/com/android/tools/lint/detector/api/LintConstants.java @@ -349,6 +349,10 @@ public class LintConstants { public static final String ANDROID_CONTENT_BROADCAST_RECEIVER = "android/content/BroadcastReceiver"; //$NON-NLS-1$ + // Method Names + public static final String FORMAT_METHOD = "format"; //$NON-NLS-1$ + public static final String GET_STRING_METHOD = "getString"; //$NON-NLS-1$ + /** * The highest known API level. Note that the tools may also look at the * installed platforms to see if they can find more recently released diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java index 27073e7..8e4ab9d 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java @@ -54,7 +54,7 @@ public class BuiltinIssueRegistry extends IssueRegistry { private static final List<Issue> sIssues; static { - final int initialCapacity = 103; + final int initialCapacity = 104; List<Issue> issues = new ArrayList<Issue>(initialCapacity); issues.add(AccessibilityDetector.ISSUE); @@ -86,6 +86,7 @@ public class BuiltinIssueRegistry extends IssueRegistry { issues.add(GridLayoutDetector.ISSUE); issues.add(OnClickDetector.ISSUE); issues.add(ViewTagDetector.ISSUE); + issues.add(LocaleDetector.ISSUE); issues.add(RegistrationDetector.ISSUE); issues.add(HandlerDetector.ISSUE); issues.add(FragmentDetector.ISSUE); diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/LocaleDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/LocaleDetector.java new file mode 100644 index 0000000..5de8dc6 --- /dev/null +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/LocaleDetector.java @@ -0,0 +1,175 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * 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.android.tools.lint.checks; + +import static com.android.tools.lint.detector.api.LintConstants.FORMAT_METHOD; + +import com.android.annotations.NonNull; +import com.android.annotations.Nullable; +import com.android.tools.lint.detector.api.Category; +import com.android.tools.lint.detector.api.ClassContext; +import com.android.tools.lint.detector.api.Detector; +import com.android.tools.lint.detector.api.Detector.ClassScanner; +import com.android.tools.lint.detector.api.Issue; +import com.android.tools.lint.detector.api.Location; +import com.android.tools.lint.detector.api.Scope; +import com.android.tools.lint.detector.api.Severity; +import com.android.tools.lint.detector.api.Speed; + +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.InsnList; +import org.objectweb.asm.tree.LdcInsnNode; +import org.objectweb.asm.tree.MethodInsnNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.analysis.Analyzer; +import org.objectweb.asm.tree.analysis.AnalyzerException; +import org.objectweb.asm.tree.analysis.Frame; +import org.objectweb.asm.tree.analysis.SourceInterpreter; +import org.objectweb.asm.tree.analysis.SourceValue; + +import java.util.Arrays; +import java.util.EnumSet; +import java.util.List; + +/** + * Checks for errors related to locale handling + */ +public class LocaleDetector extends Detector implements ClassScanner { + /** Calling risky convenience methods */ + public static final Issue ISSUE = Issue.create( + "DefaultLocale", //$NON-NLS-1$ + "Finds calls to locale-ambiguous String manipulation methods", + + "Calling `String#toLowerCase()` or `#toUpperCase()` *without specifying an " + + "explicit locale* is a common source of bugs. The reason for that is that those " + + "methods will use the current locale on the user's device, and even though the " + + "code appears to work correctly when you are developing the app, it will fail " + + "in some locales. For example, in the Turkish locale, the uppercase replacement " + + "for `i` is *not* `I`.\n" + + "\n" + + "If you want the methods to just perform ASCII replacement, for example to convert " + + "an enum name, call `String#toUpperCase(Locale.US)` instead. If you really want to " + + "use the current locale, call `String#toUpperCase(Locale.getDefault())` instead.", + + Category.CORRECTNESS, + 6, + Severity.WARNING, + LocaleDetector.class, + EnumSet.of(Scope.ALL_RESOURCE_FILES, Scope.CLASS_FILE)).setMoreInfo( + "http://developer.android.com/reference/java/util/Locale.html#default_locale"); //$NON-NLS-1$ + + /** Constructs a new {@link LocaleDetector} */ + public LocaleDetector() { + } + + @Override + public @NonNull Speed getSpeed() { + return Speed.FAST; + } + + // ---- Implements ClassScanner ---- + + @Override + @Nullable + public List<String> getApplicableCallNames() { + return Arrays.asList( + "toLowerCase", //$NON-NLS-1$ + "toUpperCase", //$NON-NLS-1$ + FORMAT_METHOD + ); + } + + @Override + public void checkCall(@NonNull ClassContext context, @NonNull ClassNode classNode, + @NonNull MethodNode method, @NonNull MethodInsnNode call) { + String owner = call.owner; + if (!owner.equals("java/lang/String")) { //$NON-NLS-1$ + return; + } + + String desc = call.desc; + String name = call.name; + if (name.equals(FORMAT_METHOD)) { + // Only check the non-locale version of String.format + if (!desc.equals("(Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/String;")) { //$NON-NLS-1$ + return; + } + // Find the formatting string + Analyzer analyzer = new Analyzer(new SourceInterpreter() { + @Override + public SourceValue newOperation(AbstractInsnNode insn) { + if (insn.getOpcode() == Opcodes.LDC) { + Object cst = ((LdcInsnNode) insn).cst; + if (cst instanceof String) { + return new StringValue(1, (String) cst); + } + } + return super.newOperation(insn); + } + }); + try { + Frame[] frames = analyzer.analyze(classNode.name, method); + InsnList instructions = method.instructions; + Frame frame = frames[instructions.indexOf(call)]; + if (frame.getStackSize() == 0) { + return; + } + SourceValue stackValue = (SourceValue) frame.getStack(0); + if (stackValue instanceof StringValue) { + String format = ((StringValue) stackValue).getString(); + if (format != null && StringFormatDetector.isLocaleSpecific(format)) { + Location location = context.getLocation(call); + String message = + "Implicitly using the default locale is a common source of bugs: " + + "Use String.format(Locale, ...) instead"; + context.report(ISSUE, method, location, message, null); + } + } + } catch (AnalyzerException e) { + context.log(e, null); + } + } else { + if (desc.equals("()Ljava/lang/String;")) { //$NON-NLS-1$ + Location location = context.getLocation(call); + String message = String.format( + "Implicitly using the default locale is a common source of bugs: " + + "Use %1$s(Locale) instead", name); + context.report(ISSUE, method, location, message, null); + } + } + } + + private class StringValue extends SourceValue { + private final String mString; + + StringValue(int size, String string) { + super(size); + mString = string; + } + + String getString() { + return mString; + } + + @Override + public int getSize() { + return 1; + } + } +} diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecureRandomDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecureRandomDetector.java index 08f3140..4900fea 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecureRandomDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SecureRandomDetector.java @@ -40,7 +40,7 @@ import org.objectweb.asm.tree.analysis.BasicInterpreter; import org.objectweb.asm.tree.analysis.BasicValue; import org.objectweb.asm.tree.analysis.Frame; -import java.util.Arrays; +import java.util.Collections; import java.util.List; /** @@ -78,7 +78,7 @@ public class SecureRandomDetector extends Detector implements ClassScanner { @Override @Nullable public List<String> getApplicableCallNames() { - return Arrays.asList(SET_SEED); + return Collections.singletonList(SET_SEED); } @Override diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java index 23cfa38..ae1bf52 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/StringFormatDetector.java @@ -18,6 +18,8 @@ package com.android.tools.lint.checks; import static com.android.tools.lint.detector.api.LintConstants.ATTR_NAME; import static com.android.tools.lint.detector.api.LintConstants.DOT_JAVA; +import static com.android.tools.lint.detector.api.LintConstants.FORMAT_METHOD; +import static com.android.tools.lint.detector.api.LintConstants.GET_STRING_METHOD; import static com.android.tools.lint.detector.api.LintConstants.TAG_STRING; import com.android.annotations.NonNull; @@ -83,10 +85,6 @@ import lombok.ast.VariableReference; * TODO: Handle Resources.getQuantityString as well */ public class StringFormatDetector extends ResourceXmlDetector implements Detector.JavaScanner { - /** The name of the String.format method */ - private static final String FORMAT_METHOD = "format"; //$NON-NLS-1$ - private static final String GET_STRING_METHOD = "getString"; //$NON-NLS-1$ - /** Whether formatting strings are invalid */ public static final Issue INVALID = Issue.create( "StringFormatInvalid", //$NON-NLS-1$ @@ -769,6 +767,66 @@ public class StringFormatDetector extends ResourceXmlDetector implements Detecto return max; } + /** + * Determines whether the given {@link String#format(String, Object...)} + * formatting string is "locale dependent", meaning that its output depends + * on the locale. This is the case if it for example references decimal + * numbers of dates and times. + * + * @param format the format string + * @return true if the format is locale sensitive, false otherwise + */ + public static boolean isLocaleSpecific(@NonNull String format) { + if (format.indexOf('%') == -1) { + return false; + } + + String s = format; + Matcher matcher = FORMAT.matcher(s); + int index = 0; + int prevIndex = 0; + while (true) { + if (matcher.find(index)) { + int matchStart = matcher.start(); + // Make sure this is not an escaped '%' + for (; prevIndex < matchStart; prevIndex++) { + char c = s.charAt(prevIndex); + if (c == '\\') { + prevIndex++; + } + } + if (prevIndex > matchStart) { + // We're in an escape, ignore this result + index = prevIndex; + continue; + } + + String type = matcher.group(6); + if (!type.isEmpty()) { + char t = type.charAt(0); + + // The following formatting characters are locale sensitive: + switch (t) { + case 'd': // decimal integer + case 'e': // scientific + case 'E': + case 'f': // decimal float + case 'g': // general + case 'G': + case 't': // date/time + case 'T': + return true; + } + } + index = matcher.end(); + } else { + break; + } + } + + return false; + } + @Override public List<String> getApplicableMethodNames() { return Arrays.asList(FORMAT_METHOD, GET_STRING_METHOD); diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTagDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTagDetector.java index 0eeb448..e944427 100644 --- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTagDetector.java +++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTagDetector.java @@ -41,7 +41,7 @@ import org.objectweb.asm.tree.analysis.BasicInterpreter; import org.objectweb.asm.tree.analysis.BasicValue; import org.objectweb.asm.tree.analysis.Frame; -import java.util.Arrays; +import java.util.Collections; import java.util.EnumSet; import java.util.List; @@ -49,7 +49,7 @@ import java.util.List; * Checks for missing view tag detectors */ public class ViewTagDetector extends Detector implements ClassScanner { - /** Missing onClick handlers */ + /** Using setTag and leaking memory */ public static final Issue ISSUE = Issue.create( "ViewTag", //$NON-NLS-1$ "Finds potential leaks when using View.setTag", @@ -82,7 +82,7 @@ public class ViewTagDetector extends Detector implements ClassScanner { @Override @Nullable public List<String> getApplicableCallNames() { - return Arrays.asList("setTag"); //$NON-NLS-1$ + return Collections.singletonList("setTag"); //$NON-NLS-1$ } @Override diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LocaleDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LocaleDetectorTest.java new file mode 100644 index 0000000..d34ce36 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/LocaleDetectorTest.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * 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.android.tools.lint.checks; + +import com.android.tools.lint.detector.api.Detector; + +@SuppressWarnings("javadoc") +public class LocaleDetectorTest extends AbstractCheckTest { + @Override + protected Detector getDetector() { + return new LocaleDetector(); + } + + public void test() throws Exception { + assertEquals( + "src/test/pkg/LocaleTest.java:11: Warning: Implicitly using the default locale is a common source of bugs: Use toUpperCase(Locale) instead [DefaultLocale]\n" + + " System.out.println(\"WRONG\".toUpperCase());\n" + + " ~~~~~~~~~~~\n" + + "src/test/pkg/LocaleTest.java:16: Warning: Implicitly using the default locale is a common source of bugs: Use toLowerCase(Locale) instead [DefaultLocale]\n" + + " System.out.println(\"WRONG\".toLowerCase());\n" + + " ~~~~~~~~~~~\n" + + "src/test/pkg/LocaleTest.java:20: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %f\", 1.0f); // Implies locale\n" + + " ~~~~~~\n" + + "src/test/pkg/LocaleTest.java:21: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %1$f\", 1.0f);\n" + + " ~~~~~~\n" + + "src/test/pkg/LocaleTest.java:22: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %e\", 1.0f);\n" + + " ~~~~~~\n" + + "src/test/pkg/LocaleTest.java:23: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %d\", 1.0f);\n" + + " ~~~~~~\n" + + "src/test/pkg/LocaleTest.java:24: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %g\", 1.0f);\n" + + " ~~~~~~\n" + + "src/test/pkg/LocaleTest.java:25: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %g\", 1.0f);\n" + + " ~~~~~~\n" + + "src/test/pkg/LocaleTest.java:26: Warning: Implicitly using the default locale is a common source of bugs: Use String.format(Locale, ...) instead [DefaultLocale]\n" + + " String.format(\"WRONG: %1$tm %1$te,%1$tY\",\n" + + " ~~~~~~\n" + + "0 errors, 9 warnings\n", + + lintProject( + "bytecode/.classpath=>.classpath", + "bytecode/AndroidManifest.xml=>AndroidManifest.xml", + "res/layout/onclick.xml=>res/layout/onclick.xml", + "bytecode/LocaleTest.java.txt=>src/test/pkg/LocaleTest.java", + "bytecode/LocaleTest.class.data=>bin/classes/test/pkg/LocaleTest.class" + )); + } +} diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java index 0ffaa08..5995a8c 100644 --- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/StringFormatDetectorTest.java @@ -16,6 +16,8 @@ package com.android.tools.lint.checks; +import static com.android.tools.lint.checks.StringFormatDetector.isLocaleSpecific; + import com.android.tools.lint.detector.api.Detector; import java.util.HashSet; @@ -151,4 +153,18 @@ public class StringFormatDetectorTest extends AbstractCheckTest { lintProject("res/values/formatstrings3.xml")); } + + public void testIsLocaleSpecific() throws Exception { + assertFalse(isLocaleSpecific("")); + assertFalse(isLocaleSpecific("Hello World!")); + assertFalse(isLocaleSpecific("%% %n")); + assertFalse(isLocaleSpecific(" %%f")); + assertFalse(isLocaleSpecific("%x %A %c %b %B %h %n %%")); + assertTrue(isLocaleSpecific("%f")); + assertTrue(isLocaleSpecific(" %1$f ")); + assertTrue(isLocaleSpecific(" %5$e ")); + assertTrue(isLocaleSpecific(" %E ")); + assertTrue(isLocaleSpecific(" %g ")); + assertTrue(isLocaleSpecific(" %1$tm %1$te,%1$tY ")); + } } diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/LocaleTest.class.data b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/LocaleTest.class.data Binary files differnew file mode 100644 index 0000000..542cd70 --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/LocaleTest.class.data diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/LocaleTest.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/LocaleTest.java.txt new file mode 100644 index 0000000..0494f0e --- /dev/null +++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/bytecode/LocaleTest.java.txt @@ -0,0 +1,29 @@ +package test.pkg; + +import java.util.GregorianCalendar; +import java.util.Locale; + +public class LocaleTest { + public void test() { + System.out.println("OK".toUpperCase(Locale.getDefault())); + System.out.println("OK".toUpperCase(Locale.US)); + System.out.println("OK".toUpperCase(Locale.CHINA)); + System.out.println("WRONG".toUpperCase()); + + System.out.println("OK".toLowerCase(Locale.getDefault())); + System.out.println("OK".toLowerCase(Locale.US)); + System.out.println("OK".toLowerCase(Locale.CHINA)); + System.out.println("WRONG".toLowerCase()); + + String.format(Locale.getDefault(), "OK: %f", 1.0f); + String.format("OK: %x %A %c %b %B %h %n %%", 1, 2, 'c', true, false, 5); + String.format("WRONG: %f", 1.0f); // Implies locale + String.format("WRONG: %1$f", 1.0f); + String.format("WRONG: %e", 1.0f); + String.format("WRONG: %d", 1.0f); + String.format("WRONG: %g", 1.0f); + String.format("WRONG: %g", 1.0f); + String.format("WRONG: %1$tm %1$te,%1$tY", + new GregorianCalendar(2012, GregorianCalendar.AUGUST, 27)); + } +} |