Fix nits from PR

- squash commits - revert composer.json - hackfmt, tabs to spaces
hhvm · Oct 23, 2020 · a9a3ae8 · a9a3ae8
1 parent 6bf55f1
commit a9a3ae8
Show file tree

Hide file tree

Showing 13 changed files with 184 additions and 149 deletions.
diff --git a/composer.json b/composer.json
@@ -20,10 +20,10 @@
     },
     "require": {
         "hhvm": "^4.80",
-        "hhvm/hsl": "^4.41",
+        "hhvm/hsl": "^4.25",
         "hhvm/hsl-experimental": "^4.58.0rc1",
         "hhvm/hsl-io": "^0.2.0",
-        "hhvm/hhvm-autoload": "^3.1",
+        "hhvm/hhvm-autoload": "^2.0.4|^3.0",
         "hhvm/type-assert": "^3.0|^4.0",
         "facebook/hh-clilib": "^2.5.0rc1",
         "facebook/difflib": "^1.0.0"

diff --git a/src/Linters/NamespacePrivateLinter.hack b/src/Linters/NamespacePrivateLinter.hack
@@ -10,139 +10,175 @@
 namespace Facebook\HHAST;
 use namespace HH\Lib\{C, Str, Vec};
 final class NamespacePrivateLinter extends ASTLinter {
-	const type TContext = Script;
-	const type TNode = Script;
+  const type TContext = Script;
+  const type TNode = Script;
 
-	<<__Override>>
-	public function getLintErrorForNode(this::TContext $context, this::TNode $node): ?ASTLintError {
-		// Process all namespaces, use statements present in the file.
-		$namespaces_details = $node->getNamespaces();
+  <<__Override>>
+  public function getLintErrorForNode(
+    this::TContext $context,
+    this::TNode $node,
+  ): ?ASTLintError {
+    // Process all namespaces, use statements present in the file.
+    $namespaces_details = $node->getNamespaces();
 
-		// Go through namespace details and its uses to check for potential violations of private namespace boundaries.
-		foreach ($namespaces_details as $namespace_detail_shape) {
-			$current_namespace = $namespace_detail_shape['name'] ?? '';
-			$namespace_uses = Shapes::toArray($namespace_detail_shape['uses']);
+    // Go through namespace details and its uses to check for potential violations of private namespace boundaries.
+    foreach ($namespaces_details as $namespace_detail_shape) {
+      $current_namespace = $namespace_detail_shape['name'] ?? '';
+      $namespace_uses = Shapes::toArray($namespace_detail_shape['uses']);
 
-			foreach ($namespace_uses as $namespace_use_dict) {
-				foreach ($namespace_use_dict as $namespace_path) {
-					if (!$this->isPrivateNamespacePathAllowed($namespace_path, $current_namespace)) {
-						return new ASTLintError(
-							$this,
-							'Artifacts belonging to a namespace are private to the parent namespace',
-							$node,
-						);
-					}
-				}
-			}
-		}
+      foreach ($namespace_uses as $namespace_use_dict) {
+        foreach ($namespace_use_dict as $namespace_path) {
+          if (
+            !$this->isPrivateNamespacePathAllowed(
+              $namespace_path,
+              $current_namespace,
+            )
+          ) {
+            return new ASTLintError(
+              $this,
+              'Artifacts belonging to a namespace are private to the parent namespace',
+              $node,
+            );
+          }
+        }
+      }
+    }
 
-		$traversed_qualified_names = varray[];
-		foreach ($node->getDescendantsOfType(QualifiedName::class) as $qualified_name_token) {
-			$name_token_key = '';
-			if ($qualified_name_token->hasParts()) {
-				$name_token_key = $qualified_name_token->getDescendantsOfType(NameToken::class)
-					|> Vec\map($$, $qualified_name_token ==> $qualified_name_token->getText())
-					|> Str\join($$, '\\');
-				if (qualified_name_is_fully_qualified($qualified_name_token)) {
-					$name_token_key = '\\'.$name_token_key;
-				}
-			}
+    $traversed_qualified_names = varray[];
+    foreach (
+      $node->getDescendantsOfType(QualifiedName::class) as $qualified_name_token
+    ) {
+      $name_token_key = '';
+      if ($qualified_name_token->hasParts()) {
+        $name_token_key = $qualified_name_token->getDescendantsOfType(
+          NameToken::class,
+        )
+          |> Vec\map(
+            $$,
+            $qualified_name_token ==> $qualified_name_token->getText(),
+          )
+          |> Str\join($$, '\\');
+        if (qualified_name_is_fully_qualified($qualified_name_token)) {
+          $name_token_key = '\\'.$name_token_key;
+        }
+      }
 
-			// If a qualified token has already been traversed or has no parts, then skip it.
-      		if (Str\is_empty($name_token_key) || C\contains($traversed_qualified_names, $name_token_key)) {
-				continue;
-			}
-			// Add the current qualified name token to the traversed list.
-			$traversed_qualified_names[] = $name_token_key;
+      // If a qualified token has already been traversed or has no parts, then skip it.
+      if (
+        Str\is_empty($name_token_key) ||
+        C\contains($traversed_qualified_names, $name_token_key)
+      ) {
+        continue;
+      }
+      // Add the current qualified name token to the traversed list.
+      $traversed_qualified_names[] = $name_token_key;
 
-			// get the namespace for each qualified name token that we encounter.
-			foreach ($namespaces_details as $ns) {
-				if ($ns['children']->isAncestorOf($qualified_name_token)) {
-					$current_namespace = $ns['name'] ?? '';
-					$fully_qualified_name_for_current_token = '';
+      // get the namespace for each qualified name token that we encounter.
+      foreach ($namespaces_details as $ns) {
+        if ($ns['children']->isAncestorOf($qualified_name_token)) {
+          $current_namespace = $ns['name'] ?? '';
+          $fully_qualified_name_for_current_token = '';
 
-					$parent_node = $context->getParentOfDescendant($qualified_name_token);
-					if ($parent_node is ScopeResolutionExpression || $parent_node is NameExpression) {
-						$fully_qualified_name_for_current_token = $this->resolveScope(
-							$name_token_key,
-							$ns['uses']['namespaces'],
-							$current_namespace
-						);
-					} else if ($parent_node is FunctionCallExpression) {
-						$fully_qualified_name_for_current_token = resolve_function(
-							$name_token_key,
-							$context,
-							$qualified_name_token
-						);
-					} else if ($parent_node is SimpleTypeSpecifier) {
-						$fully_qualified_name_for_current_token = resolve_type(
-							$name_token_key,
-							$context,
-							$qualified_name_token
-						)['name'];
-					}
-					/**
-					 * In some cases, the current namespace is also a qualified name, so it gets filtered into a qualified name token.
-					 * We do not want to check for private keyword in that case.
-					 */
-					if ($fully_qualified_name_for_current_token === $current_namespace) {
-						continue;
-					}
+          $parent_node = $context->getParentOfDescendant($qualified_name_token);
+          if (
+            $parent_node is ScopeResolutionExpression ||
+            $parent_node is NameExpression
+          ) {
+            $fully_qualified_name_for_current_token = $this->resolveScope(
+              $name_token_key,
+              $ns['uses']['namespaces'],
+              $current_namespace,
+            );
+          } else if ($parent_node is FunctionCallExpression) {
+            $fully_qualified_name_for_current_token = resolve_function(
+              $name_token_key,
+              $context,
+              $qualified_name_token,
+            );
+          } else if ($parent_node is SimpleTypeSpecifier) {
+            $fully_qualified_name_for_current_token = resolve_type(
+              $name_token_key,
+              $context,
+              $qualified_name_token,
+            )['name'];
+          }
+          /**
+           * In some cases, the current namespace is also a qualified name, so it gets filtered into a qualified name token.
+           * We do not want to check for private keyword in that case.
+           */
+          if ($fully_qualified_name_for_current_token === $current_namespace) {
+            continue;
+          }
 
-					if (!$this->isPrivateNamespacePathAllowed($fully_qualified_name_for_current_token, $current_namespace)) {
-						return new ASTLintError(
-							$this,
-							'Artifacts belonging to a namespace are private to the parent namespace',
-							$qualified_name_token,
-						);
-					}
-				}
-			}
-		}
-		return null;
-	}
+          if (
+            !$this->isPrivateNamespacePathAllowed(
+              $fully_qualified_name_for_current_token,
+              $current_namespace,
+            )
+          ) {
+            return new ASTLintError(
+              $this,
+              'Artifacts belonging to a namespace are private to the parent namespace',
+              $qualified_name_token,
+            );
+          }
+        }
+      }
+    }
+    return null;
+  }
 
-	/**
-	 * A function to resolve scope resolution expression and hhast expression. It uses the dictionary of
-   	 * namespace statements in file to resolve the full path. If the path is not found in the dictionary,
-   	 * then add the current namespace to the name token.
-	 */
-	private function resolveScope(string $name_token_key, dict<string, string> $namespaces, string $current_namespace): string {
-		if (Str\search($name_token_key, '\\') === 0) {
-			return Str\slice($name_token_key, 1);
-		}
-		$name_token_key_parts = Str\split($name_token_key, '\\');
-		foreach ($namespaces as $namespace_key => $namespace_full_path) {
-			if ($namespace_key === $name_token_key_parts[C\count($name_token_key_parts) - 1]) {
-				return $namespace_full_path;
-			}
-		}
-		return $current_namespace.'\\'.$name_token_key;
-	}
+  /**
+   * A function to resolve scope resolution expression and hhast expression. It uses the dictionary of
+       * namespace statements in file to resolve the full path. If the path is not found in the dictionary,
+       * then add the current namespace to the name token.
+   */
+  private function resolveScope(
+    string $name_token_key,
+    dict<string, string> $namespaces,
+    string $current_namespace,
+  ): string {
+    if (Str\search($name_token_key, '\\') === 0) {
+      return Str\slice($name_token_key, 1);
+    }
+    $name_token_key_parts = Str\split($name_token_key, '\\');
+    foreach ($namespaces as $namespace_key => $namespace_full_path) {
+      if (
+        $namespace_key ===
+          $name_token_key_parts[C\count($name_token_key_parts) - 1]
+      ) {
+        return $namespace_full_path;
+      }
+    }
+    return $current_namespace.'\\'.$name_token_key;
+  }
 
-	/**
-	 * This method takes in a given qualified name and the current namespace for the file. It checks if
-	 * the qualified name has "private" in it, then it should be used in a similar first level parent namespace.
-	 * eg: \TeamJoins\__Private could be used in \TeamJoins or \TeamJoins\Invite namespaces.
-	 */
-	private function isPrivateNamespacePathAllowed(string $namespace_path, string $current_file_namespace): bool {
-		$namespace_path_parts = Str\split($namespace_path, '\\');
-		$current_file_namespace_parts = Str\split($current_file_namespace, '\\');
+  /**
+   * This method takes in a given qualified name and the current namespace for the file. It checks if
+   * the qualified name has "private" in it, then it should be used in a similar first level parent namespace.
+   * eg: \TeamJoins\__Private could be used in \TeamJoins or \TeamJoins\Invite namespaces.
+   */
+  private function isPrivateNamespacePathAllowed(
+    string $namespace_path,
+    string $current_file_namespace,
+  ): bool {
+    $namespace_path_parts = Str\split($namespace_path, '\\');
+    $current_file_namespace_parts = Str\split($current_file_namespace, '\\');
 
-		$private_in_method_call = C\contains($namespace_path_parts, '_Private') ||
-			C\contains($namespace_path_parts, '__Private');
-		if ($private_in_method_call) {
-			/**
-			 * If current is empty, then it implies that the private namespace artifacts are used in unrestructured world.
-       		 * We do not want that. Additionally, if the top level namespace does not match, we do not care for rest of the qualified path.
-			 */
-			if (
-				C\is_empty($current_file_namespace_parts) ||
-				$current_file_namespace_parts[0] !== $namespace_path_parts[0]
-			) {
-				return false;
-			}
-		}
-		return true;
-	}
+    $private_in_method_call = C\contains($namespace_path_parts, '_Private') ||
+      C\contains($namespace_path_parts, '__Private');
+    if ($private_in_method_call) {
+      /**
+       * If current is empty, then it implies that the private namespace artifacts are used in unrestructured world.
+       * We do not want that. Additionally, if the top level namespace does not match, we do not care for rest of the qualified path.
+       */
+      if (
+        C\is_empty($current_file_namespace_parts) ||
+        $current_file_namespace_parts[0] !== $namespace_path_parts[0]
+      ) {
+        return false;
+      }
+    }
+    return true;
+  }
 }
diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_call_method_in_another_ns.php.in b/tests/examples/NamespacePrivateLinter/namespace_private_call_method_in_another_ns.php.in
@@ -11,17 +11,16 @@
 
 namespace Something\__Private {
   class TestingWithPrivateNamespace {
-	public static function iAmBanned(): void {}
+    public static function iAmBanned(): void {}
   }
 }
 
 namespace Something;
 class TestSomething {
   function fn(): void {
-	\Something\__Private\TestingWithPrivateNamespace::iAmBanned();
+    \Something\__Private\TestingWithPrivateNamespace::iAmBanned();
   }
 }
 function test_fn(): void {
   __Private\TestingWithPrivateNamespace::iAmBanned();
 }
-
diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_call_non_static_private.php.in b/tests/examples/NamespacePrivateLinter/namespace_private_call_non_static_private.php.in
@@ -10,8 +10,8 @@ namespace NewTesting;
 
 class NewTesting {
 
-  public static function iAmBanned() : void {
-	$obj = new \Something\__Private\TestingWithPrivateNamespace();
-	$obj->testing();
+  public static function iAmBanned(): void {
+    $obj = new \Something\__Private\TestingWithPrivateNamespace();
+    $obj->testing();
   }
 }
diff --git a/...s/examples/NamespacePrivateLinter/namespace_private_call_non_static_private_method.php.in b/...s/examples/NamespacePrivateLinter/namespace_private_call_non_static_private_method.php.in
@@ -11,6 +11,6 @@
 
 class Testing {
   public static function iAmBanned(): void {
-	(new \Something\__Private\TestingWithPrivateNamespace())->testing();
+    (new \Something\__Private\TestingWithPrivateNamespace())->testing();
   }
 }
diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference.php.in b/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference.php.in
@@ -8,6 +8,6 @@
  */
 final class Test {
   private static function testThis() {
-	$some_value as SomeNamespace\__Private\SomeEnumName;
+    $some_value as SomeNamespace\__Private\SomeEnumName;
   }
 }
diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference_in_same_ws.php.in b/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference_in_same_ws.php.in
@@ -12,10 +12,10 @@ use namespace TeamJoins\Invite;
 
 final class Test {
   public function testMe() {
-	$some_value as \SomeNamespace\__Private\SomeEnumName;
-	\SomeNamespace\__Private\SomeEnumName::SOME_VALUE;
-	Invite::SOME_VALUE;
-	Invite\MyInvite::SOME_VALUE;
-	wants_classname(__Private\SomeClass::class);
+    $some_value as \SomeNamespace\__Private\SomeEnumName;
+    \SomeNamespace\__Private\SomeEnumName::SOME_VALUE;
+    Invite::SOME_VALUE;
+    Invite\MyInvite::SOME_VALUE;
+    wants_classname(__Private\SomeClass::class);
   }
 }
diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference_new.php.expect b/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference_new.php.expect
@@ -1,7 +1,7 @@
 [
     {
-        "blame": "\t\\SomeNamespace\\__Private\\SomeEnumName",
-        "blame_pretty": "\t\\SomeNamespace\\__Private\\SomeEnumName",
+        "blame": "    \\SomeNamespace\\__Private\\SomeEnumName",
+        "blame_pretty": "    \\SomeNamespace\\__Private\\SomeEnumName",
         "description": "Artifacts belonging to a namespace are private to the parent namespace"
     }
 ]
diff --git a/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference_new.php.in b/tests/examples/NamespacePrivateLinter/namespace_private_enum_reference_new.php.in
@@ -8,6 +8,6 @@
  */
 final class Test {
   private static function testThis() {
-	\SomeNamespace\__Private\SomeEnumName::SOME_VALUE;
+    \SomeNamespace\__Private\SomeEnumName::SOME_VALUE;
   }
 }