A tiny clean-up of JsMessageVisitor to make a followup change clearer.

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=233983013
google · Feb 15, 2019 · 2f7d196 · 2f7d196
1 parent 2118265
commit 2f7d196
Showing 1 changed file with 46 additions and 40 deletions.
diff --git a/src/com/google/javascript/jscomp/JsMessageVisitor.java b/src/com/google/javascript/jscomp/JsMessageVisitor.java
@@ -182,60 +182,58 @@ public void process(Node externs, Node root) {
   }
 
   @Override
-  public void visit(NodeTraversal traversal, Node node, Node parent) {
+  public void visit(NodeTraversal traversal, Node node, Node unused) {
+    collectGetMsgCall(traversal, node);
+    checkMessageInitialization(traversal, node);
+  }
+
+  private void checkMessageInitialization(NodeTraversal traversal, Node node) {
+    final Node parent = node.getParent();
+
     String messageKey;
-    String originalMessageKey;
+    final String originalMessageKey;
-    boolean isVar;
+    final Node msgNode;
-    Node msgNode;
+    final boolean isVar;
 
     switch (node.getToken()) {
       case NAME:
-        // var MSG_HELLO = 'Message'
+        // Case: `var MSG_HELLO = 'Message';`
-        if ((parent != null) && (NodeUtil.isNameDeclaration(parent))) {
+        if (parent == null || !NodeUtil.isNameDeclaration(parent)) {
-          messageKey = node.getString();
-          originalMessageKey = node.getOriginalName();
-          isVar = true;
-        } else {
           return;
         }
 
+        messageKey = node.getString();
+        originalMessageKey = node.getOriginalName();
         msgNode = node.getFirstChild();
+        isVar = true;
         break;
-      case ASSIGN:
-        // somenamespace.someclass.MSG_HELLO = 'Message'
-        isVar = false;
 
+      case ASSIGN:
+        // Case: `somenamespace.someclass.MSG_HELLO = 'Message';`
         Node getProp = node.getFirstChild();
         if (!getProp.isGetProp()) {
           return;
         }
 
-        Node propNode = getProp.getLastChild();
+        messageKey = getProp.getLastChild().getString();
-
-        messageKey = propNode.getString();
         originalMessageKey = getProp.getOriginalName();
         msgNode = node.getLastChild();
+        isVar = false;
         break;
 
       case STRING_KEY:
+        // Case: `var t = {MSG_HELLO: 'Message'}`;
+        // Case: `var {MSG_HELLO} = x;
         if (node.isQuotedString() || !node.hasChildren()) {
           return;
         }
-        isVar = false;
+
         messageKey = node.getString();
         originalMessageKey = node.getOriginalName();
         msgNode = node.getFirstChild();
+        isVar = false;
         break;
 
-      case CALL:
-        // goog.getMsg()
-        if (node.getFirstChild().matchesQualifiedName(MSG_FUNCTION_NAME)) {
-          googMsgNodes.add(node);
-        } else if (node.getFirstChild().matchesQualifiedName(
-            MSG_FALLBACK_FUNCTION_NAME)) {
-          visitFallbackFunctionCall(traversal, node);
-        }
-        return;
       default:
         return;
     }
@@ -245,8 +243,7 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
     }
 
     // Is this a message name?
-    boolean isNewStyleMessage =
+    boolean isNewStyleMessage = msgNode != null && msgNode.isCall();
-        msgNode != null && msgNode.isCall();
     if (!isMessageName(messageKey, isNewStyleMessage)) {
       return;
     }
@@ -265,8 +262,8 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
       return;
     }
 
-    // Report a warning if a qualified messageKey that looks like a message
+    // Report a warning if a qualified messageKey that looks like a message (e.g. "a.b.MSG_X")
-    // (e.g. "a.b.MSG_X") doesn't use goog.getMsg().
+    // doesn't use goog.getMsg().
     if (isNewStyleMessage) {
       googMsgNodes.remove(msgNode);
     } else if (style != JsMessage.Style.LEGACY) {
@@ -281,11 +278,10 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
 
     boolean isUnnamedMsg = isUnnamedMessageName(messageKey);
 
-    Builder builder = new Builder(
+    JsMessage.Builder builder = new JsMessage.Builder(isUnnamedMsg ? null : messageKey);
-        isUnnamedMsg ? null : messageKey);
+    OriginalMapping mapping =
-    OriginalMapping mapping = compiler.getSourceMapping(
+        compiler.getSourceMapping(
-        traversal.getSourceName(), traversal.getLineNumber(),
+            traversal.getSourceName(), traversal.getLineNumber(), traversal.getCharno());
-        traversal.getCharno());
     if (mapping != null) {
       builder.setSourceName(mapping.getOriginalFile());
     } else {
@@ -306,13 +302,10 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
     JsMessage extractedMessage = builder.build(idGenerator);
 
     // If asked to check named internal messages.
-    if (needToCheckDuplications
+    if (needToCheckDuplications && !isUnnamedMsg && !extractedMessage.isExternal()) {
-        && !isUnnamedMsg
-        && !extractedMessage.isExternal()) {
       checkIfMessageDuplicated(messageKey, msgNode);
     }
-    trackMessage(traversal, extractedMessage,
+    trackMessage(traversal, extractedMessage, messageKey, msgNode, isUnnamedMsg);
-        messageKey, msgNode, isUnnamedMsg);
 
     if (extractedMessage.isEmpty()) {
       // value of the message is an empty string. Translators do not like it.
@@ -334,6 +327,19 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
     processJsMessage(extractedMessage, msgDefinition);
   }
 
+  private void collectGetMsgCall(NodeTraversal traversal, Node call) {
+    if (!call.isCall()) {
+      return;
+    }
+
+    // goog.getMsg()
+    if (call.getFirstChild().matchesQualifiedName(MSG_FUNCTION_NAME)) {
+      googMsgNodes.add(call);
+    } else if (call.getFirstChild().matchesQualifiedName(MSG_FALLBACK_FUNCTION_NAME)) {
+      visitFallbackFunctionCall(traversal, call);
+    }
+  }
+
   /**
    * Track a message for later retrieval.
    *