Also check SimpleDateFormat calls for explicit locale

This changeset expands on the lint check for calling String
convenience methods that are locale sensitive without an explicit
locale (issue 36822). It checks whether a SimpleDateFormat constructor
that uses the default locale is called, and if so suggests either
specifying a locale or using one of the pre-configured locale
sensitive instances of SimpleDateFormat.

Change-Id: Ic4fcb72c2ee6b31d1534b4d909493fef7dcff867

6 files changed, 79 insertions, 13 deletions

diff --git a/lint/cli/src/com/android/tools/lint/Main.java b/lint/cli/src/com/android/tools/lint/Main.java
index 47d00d2..e83601f 100644
--- a/lint/cli/src/com/android/tools/lint/Main.java
+++ b/lint/cli/src/com/android/tools/lint/Main.java
@@ -836,7 +836,7 @@ public class Main extends LintClient {
 
         if (issue.getExplanation() != null) {
             System.out.println();
-            System.out.println(wrap(issue.getExplanation()));
+            System.out.println(wrap(issue.getExplanationAsSimpleText()));
         }
         if (issue.getMoreInfo() != null) {
             System.out.println("More information: " + issue.getMoreInfo());
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 3c08e82..ac09a1b 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 = 105;
+        final int initialCapacity = 106;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -86,7 +86,8 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(GridLayoutDetector.ISSUE);
         issues.add(OnClickDetector.ISSUE);
         issues.add(ViewTagDetector.ISSUE);
-        issues.add(LocaleDetector.ISSUE);
+        issues.add(LocaleDetector.STRING_LOCALE);
+        issues.add(LocaleDetector.DATE_FORMAT);
         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
index 5de8dc6..29b7b48 100644
--- 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
@@ -16,6 +16,7 @@
 
 package com.android.tools.lint.checks;
 
+import static com.android.tools.lint.detector.api.LintConstants.CONSTRUCTOR_NAME;
 import static com.android.tools.lint.detector.api.LintConstants.FORMAT_METHOD;
 
 import com.android.annotations.NonNull;
@@ -44,6 +45,7 @@ import org.objectweb.asm.tree.analysis.SourceInterpreter;
 import org.objectweb.asm.tree.analysis.SourceValue;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
 
@@ -52,7 +54,7 @@ import java.util.List;
  */
 public class LocaleDetector extends Detector implements ClassScanner {
     /** Calling risky convenience methods */
-    public static final Issue ISSUE = Issue.create(
+    public static final Issue STRING_LOCALE = Issue.create(
             "DefaultLocale", //$NON-NLS-1$
             "Finds calls to locale-ambiguous String manipulation methods",
 
@@ -74,6 +76,32 @@ public class LocaleDetector extends Detector implements ClassScanner {
             EnumSet.of(Scope.ALL_RESOURCE_FILES, Scope.CLASS_FILE)).setMoreInfo(
              "http://developer.android.com/reference/java/util/Locale.html#default_locale"); //$NON-NLS-1$
 
+    /** Constructing SimpleDateFormat without an explicit locale */
+    public static final Issue DATE_FORMAT = Issue.create(
+            "SimpleDateFormat", //$NON-NLS-1$
+            "Using SimpleDateFormat directly without an explicit locale",
+
+            "Almost all callers should use `getDateInstance()`, `getDateTimeInstance()`, or " +
+            "`getTimeInstance()` to get a ready-made instance of SimpleDateFormat suitable " +
+            "for the user's locale. The main reason you'd create an instance this class " +
+            "directly is because you need to format/parse a specific machine-readable format, " +
+            "in which case you almost certainly want to explicitly ask for US to ensure that " +
+            "you get ASCII digits (rather than, say, Arabic digits).\n" +
+            "\n" +
+            "Therefore, you should either use the form of the SimpleDateFormat constructor " +
+            "where you pass in an explicit locale, such as Locale.US, or use one of the " +
+            "get instance methods, or suppress this error if really know what you are doing.",
+
+            Category.CORRECTNESS,
+            6,
+            Severity.WARNING,
+            LocaleDetector.class,
+            EnumSet.of(Scope.ALL_RESOURCE_FILES, Scope.CLASS_FILE)).setMoreInfo(
+             "http://developer.android.com/reference/java/text/SimpleDateFormat.html"); //$NON-NLS-1$
+
+    private static final String DATE_FORMAT_OWNER = "java/text/SimpleDateFormat"; //$NON-NLS-1$
+    private static final String STRING_OWNER = "java/lang/String";                //$NON-NLS-1$
+
     /** Constructs a new {@link LocaleDetector} */
     public LocaleDetector() {
     }
@@ -96,15 +124,35 @@ public class LocaleDetector extends Detector implements ClassScanner {
     }
 
     @Override
+    @Nullable
+    public List<String> getApplicableCallOwners() {
+        return Collections.singletonList(DATE_FORMAT_OWNER);
+    }
+
+    @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$
+        String desc = call.desc;
+        String name = call.name;
+        if (owner.equals(DATE_FORMAT_OWNER)) {
+            if (!name.equals(CONSTRUCTOR_NAME)) {
+                return;
+            }
+            if (desc.equals("(Ljava/lang/String;Ljava/text/DateFormatSymbols;)V")   //$NON-NLS-1$
+                    || desc.equals("()V")                                           //$NON-NLS-1$
+                    || desc.equals("(Ljava/lang/String;)V")) {                      //$NON-NLS-1$
+                Location location = context.getLocation(call);
+                String message = String.format(
+                    "To get local formatting use getDateInstance(), getDateTimeInstance(), " +
+                    "or getTimeInstance(), or use new SimpleDateFormat(String template, " +
+                    "Locale locale) with for example Locale.US for ASCII dates.", name);
+                context.report(DATE_FORMAT, method, location, message, null);
+            }
+        } else if (!owner.equals(STRING_OWNER)) {
             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$
@@ -138,7 +186,7 @@ public class LocaleDetector extends Detector implements ClassScanner {
                         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);
+                        context.report(STRING_LOCALE, method, location, message, null);
                     }
                 }
             } catch (AnalyzerException e) {
@@ -150,7 +198,7 @@ public class LocaleDetector extends Detector implements ClassScanner {
                 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);
+                context.report(STRING_LOCALE, method, location, message, null);
             }
         }
     }
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
index d34ce36..2ac3c4e 100644
--- 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
@@ -54,7 +54,16 @@ public class LocaleDetectorTest extends AbstractCheckTest {
             "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",
+            "src/test/pkg/LocaleTest.java:32: Warning: To get local formatting use getDateInstance(), getDateTimeInstance(), or getTimeInstance(), or use new SimpleDateFormat(String template, Locale locale) with for example Locale.US for ASCII dates. [SimpleDateFormat]\n" +
+            "        new SimpleDateFormat(); // WRONG\n" +
+            "            ~~~~~~~~~~~~~~~~\n" +
+            "src/test/pkg/LocaleTest.java:33: Warning: To get local formatting use getDateInstance(), getDateTimeInstance(), or getTimeInstance(), or use new SimpleDateFormat(String template, Locale locale) with for example Locale.US for ASCII dates. [SimpleDateFormat]\n" +
+            "        new SimpleDateFormat(\"yyyy-MM-dd\"); // WRONG\n" +
+            "            ~~~~~~~~~~~~~~~~\n" +
+            "src/test/pkg/LocaleTest.java:34: Warning: To get local formatting use getDateInstance(), getDateTimeInstance(), or getTimeInstance(), or use new SimpleDateFormat(String template, Locale locale) with for example Locale.US for ASCII dates. [SimpleDateFormat]\n" +
+            "        new SimpleDateFormat(\"yyyy-MM-dd\", DateFormatSymbols.getInstance()); // WRONG\n" +
+            "            ~~~~~~~~~~~~~~~~\n" +
+            "0 errors, 12 warnings\n",
 
             lintProject(
                     "bytecode/.classpath=>.classpath",
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
index 542cd70..2b10aa9 100644
--- 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
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
index 0494f0e..3c60c9d 100644
--- 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
@@ -1,10 +1,10 @@
 package test.pkg;
 
-import java.util.GregorianCalendar;
-import java.util.Locale;
+import java.text.*;
+import java.util.*;
 
 public class LocaleTest {
-    public void test() {
+    public void testStrings() {
         System.out.println("OK".toUpperCase(Locale.getDefault()));
         System.out.println("OK".toUpperCase(Locale.US));
         System.out.println("OK".toUpperCase(Locale.CHINA));
@@ -26,4 +26,12 @@ public class LocaleTest {
         String.format("WRONG: %1$tm %1$te,%1$tY",
                 new GregorianCalendar(2012, GregorianCalendar.AUGUST, 27));
     }
+
+    @android.annotation.SuppressLint("NewApi") // DateFormatSymbols requires API 9
+    public void testSimpleDateFormat() {
+        new SimpleDateFormat(); // WRONG
+        new SimpleDateFormat("yyyy-MM-dd"); // WRONG
+        new SimpleDateFormat("yyyy-MM-dd", DateFormatSymbols.getInstance()); // WRONG
+        new SimpleDateFormat("yyyy-MM-dd", Locale.US); // OK
+    }
 }