[DevTools] Disallow use of global property "document".

Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/DisallowedGlobalPropertyChecker.java

Index: Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/DisallowedGlobalPropertyChecker.java
diff --git a/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/DisallowedGlobalPropertyChecker.java b/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/DisallowedGlobalPropertyChecker.java
new file mode 100644
index 0000000000000000000000000000000000000000..0972a32ef02a9a3c9c8c4f550da49eddebc1261e
--- /dev/null
+++ b/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/DisallowedGlobalPropertyChecker.java
@@ -0,0 +1,168 @@
+package org.chromium.devtools.jsdoc.checks;
+
+import com.google.javascript.jscomp.NodeUtil;
+import com.google.javascript.rhino.JSDocInfo;
+import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.Token;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+public final class DisallowedGlobalPropertyChecker extends ContextTrackingChecker {

[apavlov, 2014/10/17 11:59:41]

...PropertiesChecker

[dgozman, 2014/10/17 13:05:40]

Done.

+
+    private static final String GLOBAL_OBJECT_NAME = "window";

[apavlov, 2014/10/17 11:59:41]

we've also got "self"...

[dgozman, 2014/10/17 13:05:39]

Done.

+    private static final Set<String> DISALLOWED_PROPERTIES = new HashSet<>();
+    static {
+        DISALLOWED_PROPERTIES.add("document");
+    }
+
+    private final Map<FunctionRecord, Set<String>> declaredLocalVariables = new HashMap<>();
+    private final Map<FunctionRecord, List<Node>> globalPropertyAccessNodes = new HashMap<>();
+    private final Set<String> topLevelDeclaredVariables = new HashSet<>();
+    private final List<Node> topLevelGlobalPropertyAccessNodes = new ArrayList<>();
+
+    @Override
+    protected void enterNode(Node node) {
+        switch (node.getType()) {
+        case Token.VAR:
+            handleVar(node);
+            break;
+        case Token.NAME:
+            handleName(node);
+            break;
+        case Token.STRING:
+            handleString(node);
+            break;
+        case Token.FUNCTION:
+            enterFunction();
+            break;
+        default:
+            break;
+        }
+    }
+
+    @Override
+    protected void leaveNode(Node node) {
+        if (node.getType() == Token.FUNCTION) {

[apavlov, 2014/10/17 11:59:41]

you could use a similar switch for consistency here

[dgozman, 2014/10/17 13:05:40]

Done.

+            leaveFunction();
+            return;
+        }
+        if (node.getType() == Token.SCRIPT) {
+            checkAccessNodes(topLevelGlobalPropertyAccessNodes);
+            return;
+        }
+    }
+
+    private void enterFunction() {
+        FunctionRecord function = getState().getCurrentFunctionRecord();
+        declaredLocalVariables.put(function, new HashSet<String>());
+        globalPropertyAccessNodes.put(function, new ArrayList<Node>());
+    }
+
+    private void leaveFunction() {
+        FunctionRecord function = getState().getCurrentFunctionRecord();
+        if (!function.suppressesGlobalPropertiesCheck()) {
+            checkAccessNodes(globalPropertyAccessNodes.get(function));
+        }
+        declaredLocalVariables.remove(function);
+        globalPropertyAccessNodes.remove(function);
+    }
+
+    private void checkAccessNodes(List<Node> nodes) {
+        FunctionRecord function = getState().getCurrentFunctionRecord();
+        for (Node node : nodes) {
+            String name = getContext().getNodeText(node);
+            if (!functionHasVisibleIdentifier(function, name)) {
+                reportErrorAtNodeStart(node,
+                        String.format("\"%s\" global property is disallowed", name));

[apavlov, 2014/10/17 11:59:41]

global property access is disallowed

[dgozman, 2014/10/17 13:05:40]

Done.

+            }
+        }
+    }
+
+    private void handleVar(Node varNode) {
+        Node nameNode = varNode.getFirstChild();
+        if (nameNode != null) {

[apavlov, 2014/10/17 11:59:41]

early bailout:
if (nameNode == null) {
    return;
}

[dgozman, 2014/10/17 13:05:40]

Done.

+            String name = getContext().getNodeText(nameNode);
+            if (name != null)
+                getLocalVariables(getState().getCurrentFunctionRecord()).add(name);
+        }
+    }
+
+    private void handleName(Node nameNode) {
+        Node parent = nameNode.getParent();
+        if (parent != null && parent.getType() == Token.FUNCTION) {
+            return;
+        }
+
+        String name = getContext().getNodeText(nameNode);
+        if (!DISALLOWED_PROPERTIES.contains(name)) {
+            return;
+        }
+
+        if (parent != null && parent.getType() == Token.GETPROP) {

[apavlov, 2014/10/17 11:59:41]

This conditional can be rewritten in a shorter form:

if (...) {
    boolean isGlobalPropertyAccess = ...
    if (!isGlobalPropertyAccess) {
        return
    }
}
addGlobalPropertyAccess(nameNode);

[dgozman, 2014/10/17 13:05:40]

Done.

+            boolean isGlobalPropertyAccess = parent.getFirstChild() == nameNode;
+            if (isGlobalPropertyAccess) {
+                addGlobalPropertyAccess(nameNode);
+            }
+        } else {
+            addGlobalPropertyAccess(nameNode);
+        }
+    }
+
+    private void handleString(Node stringNode) {
+        String name = getContext().getNodeText(stringNode);
+        if (!DISALLOWED_PROPERTIES.contains(name)) {
+            return;
+        }
+
+        Node parent = stringNode.getParent();
+        if (parent == null || parent.getType() != Token.GETPROP) {
+            return;
+        }
+
+        Node objectNode = parent.getFirstChild();
+        boolean isGlobalObjectAccess =
+            objectNode != null && isGlobalObject(objectNode) && objectNode.getNext() == stringNode;
+        if (isGlobalObjectAccess) {
+            addGlobalPropertyAccess(stringNode);
+        }
+    }
+
+    private Set<String> getLocalVariables(FunctionRecord function) {
+        return function == null ? topLevelDeclaredVariables : declaredLocalVariables.get(function);
+    }
+
+    private void addGlobalPropertyAccess(Node node) {
+        FunctionRecord function = getState().getCurrentFunctionRecord();
+        if (function == null) {
+            topLevelGlobalPropertyAccessNodes.add(node);

[apavlov, 2014/10/17 11:59:41]

You could use a null object pattern here (create a fake FunctionRecord for the
top level scope)

[dgozman, 2014/10/17 13:05:40]

Done.

+        } else {
+            globalPropertyAccessNodes.get(function).add(node);
+        }
+    }
+
+    private boolean isGlobalObject(Node node) {
+        String name = getContext().getNodeText(node);
+        if (!GLOBAL_OBJECT_NAME.equals(name)) {
+            return false;
+        }
+        return node.getType() == Token.NAME &&

[apavlov, 2014/10/17 11:59:41]

&& on the next line

[dgozman, 2014/10/17 13:05:40]

Done.

+            !functionHasVisibleIdentifier(
+                getState().getCurrentFunctionRecord(), name);
+    }
+
+    private boolean functionHasVisibleIdentifier(FunctionRecord function, String name) {
+        return functionHasLocalIdentifier(function, name)
+            || (function != null
+                && functionHasVisibleIdentifier(function.enclosingFunctionRecord, name));
+    }
+
+    private boolean functionHasLocalIdentifier(FunctionRecord function, String name) {
+        return (function != null && function.parameterNames.contains(name))
+            || getLocalVariables(function).contains(name);
+    }
+}