Ensure hhvm 4.40 compatibility (#40)

* Ensure hhvm 4.40 compatibility

The needs_another_plus_plus hack is terrible.
However, I must conditionally increment the pointer depending on
 - Is the LHS of the while's && false, then no.
 - Did we fall out of the loop because pointer is zero, then yes.

* Fix needs_another_plus_plus

* Fix formatting with `\t` in QueryFormatter.php

* Fix easy lint errors

* Fix a lot more difficult lint errors

.travis.yml

@@ -2,7 +2,10 @@ sudo: required
 language: generic
 services: docker
 env:
-  - HHVM_VERSION=4.8.5
+  - HHVM_VERSION=4.16-latest
+  - HHVM_VERSION=latest
+  - HHVM_VERSION=nightly
+
 install:
 - docker pull hhvm/hhvm:$HHVM_VERSION
 script:

composer.json

@@ -24,6 +24,6 @@
   "require-dev": {
     "hhvm/hhast": "^4.0.5",
     "facebook/fbexpect": "^2.2.0",
-    "hhvm/hacktest": "^1.3"
+    "hhvm/hacktest": "^1.3|^2.0"
   }
 }

src/AsyncMysql/AsyncMysqlConnectResult.php

@@ -4,30 +4,30 @@
 
 <<__MockClass>>
 final class AsyncMysqlConnectResult extends \AsyncMysqlConnectResult {
-  private int $elapsed;
-  private int $start;
+  private float $elapsed;
+  private float $start;
 
   /* HH_IGNORE_ERROR[3012] I don't want to call parent::construct */
   public function __construct(bool $from_pool) {
     // pretend connections take longer if they don't come from the pool
     if ($from_pool) {
-      $this->elapsed = 1;
+      $this->elapsed = .001;
     } else {
-      $this->elapsed = 1000;
+      $this->elapsed = .01;
     }
-    $this->start = \time();
+    $this->start = \microtime(true);
   }
 
   <<__Override>>
   public function elapsedMicros(): int {
-    return $this->elapsed;
+    return (int)($this->elapsed / 1000000);
   }
   <<__Override>>
-  public function startTime(): int {
+  public function startTime(): float {
     return $this->start;
   }
   <<__Override>>
-  public function endTime(): int {
+  public function endTime(): float {
     return $this->start + $this->elapsed;
   }
 

src/AsyncMysql/AsyncMysqlConnection.php

@@ -112,8 +112,9 @@ public function isValid(): bool {
   }
 
   <<__Override>>
-  public function serverInfo(): mixed {
-    return null;
+  public function serverInfo(): string {
+    // Copied from https://docs.hhvm.com/hack/reference/class/AsyncMysqlConnection/serverInfo/
+    return '5.6.24-fb-log-slackhq-sql-fake';
   }
 
   <<__Override>>
@@ -142,8 +143,10 @@ public function isReusable(): bool {
   }
 
   <<__Override>>
-  public function lastActivityTime(): mixed {
-    return null;
+  public function lastActivityTime(): float {
+    // A float representing the number of seconds ago since epoch that we had successful activity on the current connection.
+    // 50 ms ago seems like a reasonable answer.
+    return \microtime(true) - 0.05;
   }
 
   <<__Override>>

src/AsyncMysql/AsyncMysqlQueryResult.php

@@ -32,7 +32,7 @@ public function numRows(): int {
   }
 
   <<__Override>>
-  public function mapRows(): Vector<Map<string, string>> {
+  public function mapRows(): Vector<Map<string, ?string>> {
     $out = Vector {};
     foreach ($this->rows as $row) {
       $map = Map {};
@@ -42,11 +42,6 @@ public function mapRows(): Vector<Map<string, string>> {
       }
       $out->add($map);
     }
-    /*HH_IGNORE_ERROR[4110]
-      The actual return type of this function is `Vector<Map<string, ?string>>`
-      The parent class is typed incorrectly in the HHI file.
-      @TODO Update the return type once we target HHVM ~4.16.
-     */
     return $out;
   }
 
@@ -60,7 +55,7 @@ public function mapRowsTyped(): Vector<Map<string, mixed>> {
   }
 
   <<__Override>>
-  public function vectorRows(): Vector<Vector<string>> {
+  public function vectorRows(): Vector<KeyedContainer<int, ?string>> {
     $out = Vector {};
     foreach ($this->rows as $row) {
       $v = Vector {};
@@ -70,16 +65,11 @@ public function vectorRows(): Vector<Vector<string>> {
       }
       $out->add($v);
     }
-    /*HH_IGNORE_ERROR[4110]
-      The actual return type of this function is `Vector<Vector<?string>>`
-      The parent class is typed incorrectly in the HHI file.
-      @TODO Update the return type once we target HHVM ~4.16.
-     */
     return $out;
   }
 
   <<__Override>>
-  public function vectorRowsTyped(): Vector<Vector<mixed>> {
+  public function vectorRowsTyped(): Vector<KeyedContainer<int, mixed>> {
     $out = Vector {};
     foreach ($this->rows as $row) {
       $v = Vector {};

src/AsyncMysql/QueryFormatter.php

@@ -37,8 +37,6 @@
  */
 abstract final class QueryFormatter {
 	public static function formatQuery(string $query, mixed ...$args): string {
-		$sql = '';
-
 		// string, size, offset
 		// match types: d, f, v, s, m, u
 		// digit, float, magic, string, m..., unsigned int?
@@ -72,7 +70,8 @@ public static function formatQuery(string $query, mixed ...$args): string {
 				throw new SQLFakeParseException('too few parameters for query: %'.$c.' has no param to bind');
 			}
 
-			$param = $args[$args_pointer++];
+			$param = $args[$args_pointer];
+			$args_pointer++;
 			switch ($c) {
 				case 'd':
 				case 's':
@@ -96,7 +95,8 @@ public static function formatQuery(string $query, mixed ...$args): string {
 					$out = self::appendColumnTableName($out, $param);
 					break;
 				case '=':
-					$type = $query[++$i];
+					$i++;
+					$type = $query[$i];
 					if (!C\contains_key(keyset['d', 's', 'f', 'u'], $type)) {
 						throw new SQLFakeParseException("at %=$type, expected %=d, %=c, %=s, or %=u");
 					}
@@ -141,7 +141,8 @@ public static function formatQuery(string $query, mixed ...$args): string {
 					}
 					break;
 				case 'L':
-					$type = $query[++$i];
+					$i++;
+					$type = $query[$i];
 					if ($type === "O" || $type === "A") {
 						$out[] = "(";
 						$sep = ($type === "O") ? " OR " : " AND ";

src/BuildSchemaCLI.php

@@ -24,7 +24,6 @@ protected function getSupportedOptions(): vec<CLIOptions\CLIOption> {
 	<<__Override>>
 	public async function mainAsync(): Awaitable<int> {
 		$terminal = $this->getTerminal();
-		$stderr = $this->getStderr();
 
 		if (C\is_empty($this->getArguments())) {
 			$program = $this->getArgv()[0];

src/Expressions/BinaryOperatorExpression.php

@@ -55,7 +55,6 @@ private function evaluateRowComparison(
     }
 
     $last_index = C\last_key($left_elems);
-    $match = true;
 
     foreach ($left_elems as $index => $le) {
       $re = $right_elems[$index];

src/Expressions/CaseOperatorExpression.php

@@ -136,7 +136,7 @@ public function addRecursiveExpression(token_list $tokens, int $pointer, bool $n
 
   <<__Override>>
   public function __debugInfo(): dict<string, mixed> {
-    $last_case = C\lastx($this->whenExpressions);
+    invariant(!C\is_empty($this->whenExpressions), 'There must be at least one whenExpression');
     $when_list = vec[];
     foreach ($this->whenExpressions as $exp) {
       $when_list[] = dict['when' => \var_dump($exp['when'], true), 'then' => \var_dump($exp['then'], true)];

src/Expressions/FunctionExpression.php

@@ -398,7 +398,7 @@ private function sqlConcat(row $row, AsyncMysqlConnection $conn): string {
       throw new SQLFakeRuntimeException("MySQL CONCAT() function must be called with at least two arguments");
     }
     $final_concat = "";
-    foreach ($args as $k => $arg) {
+    foreach ($args as $arg) {
       $val = (string)$arg->evaluate($row, $conn);
       $final_concat .= $val;
     }

src/Parser/CreateTableParser.php

@@ -383,22 +383,12 @@ private function parseFieldOrKey(
 		inout vec<parsed_index> $indexes,
 	): void {
 
-		#
-		# parse a single create_definition
-		#
-
-		$has_constraint = false;
-		$constraint = null;
-
-
 		#
 		# constraints can come before a few different things
 		#
 
 		if ($tokens[0] === 'CONSTRAINT') {
 
-			$has_constraint = true;
-
 			if (
 				$tokens[1] === 'PRIMARY KEY' ||
 				$tokens[1] === 'UNIQUE' ||
@@ -409,7 +399,7 @@ private function parseFieldOrKey(
 				$tokens = Vec\drop($tokens, 1);
 			} else {
 				$tokens = Vec\drop($tokens, 1);
-				$constraint = $this->vecUnshift(inout $tokens);
+				$this->vecUnshift(inout $tokens);
 			}
 		}
 

src/Parser/ExpressionParser.php

@@ -189,7 +189,6 @@ public function tokenToExpression(token $token): Expression {
    */
   public function build(): Expression {
     $token = $this->nextToken();
-    $break_while = false;
     while ($token !== null) {
       switch ($token['type']) {
         case TokenType::PAREN:

src/Parser/FromParser.php

@@ -110,7 +110,7 @@ public function parse(): (int, FromClause) {
   private function getTableOrSubquery(token $token): from_table {
     switch ($token['type']) {
       case TokenType::IDENTIFIER:
-        return $table = shape('name' => $token['value'], 'join_type' => JoinType::JOIN);
+        return shape('name' => $token['value'], 'join_type' => JoinType::JOIN);
       case TokenType::PAREN:
         // this must be a subquery in the FROM clause
         $close = SQLParser::findMatchingParen($this->pointer, $this->tokens);

src/Parser/InsertParser.php

@@ -62,7 +62,6 @@ public function parse(): InsertQuery {
     $count = C\count($this->tokens);
 
     $needs_comma = false;
-    $end_of_set = false;
     while ($this->pointer < $count) {
       $token = $this->tokens[$this->pointer];
 
@@ -83,8 +82,13 @@ public function parse(): InsertQuery {
 
           switch ($token['value']) {
             case 'VALUES':
+              $needs_another_plus_plus = false;
               do {
                 $this->pointer++;
+                if ($needs_another_plus_plus) {
+                  $this->pointer++;
+                  $needs_another_plus_plus = false;
+                }
                 $token = $this->tokens[$this->pointer];
                 // VALUES must be followed by paren and then a list of values
                 if ($token === null || $token['value'] !== '(') {
@@ -103,7 +107,13 @@ public function parse(): InsertQuery {
                 }
                 $query->values[] = $values;
                 $this->pointer = $close;
-              } while (($this->tokens[$this->pointer + 1]['value'] ?? null) === ',' && $this->pointer++);
+                $needs_another_plus_plus = true;
+              } while (($this->tokens[$this->pointer + 1]['value'] ?? null) === ',' && $this->pointer);
+              // The while loop above used to havea $this->pointer++ here.             ^^^^^^^^^^^^^^
+              // We still need to increment is this condition is true.
+              if ($needs_another_plus_plus && ($this->tokens[$this->pointer + 1]['value'] ?? null) === ',') {
+                $this->pointer++;
+              }
               break;
             default:
               throw new SQLFakeParseException("Unexpected clause {$token['value']}");
@@ -186,7 +196,6 @@ protected function parseValues(vec<token> $tokens): vec<Expression> {
     $expressions = vec[];
 
     $needs_comma = false;
-    $end_of_set = false;
     while ($pointer < $count) {
       $token = $tokens[$pointer];
       switch ($token['type']) {
@@ -201,7 +210,6 @@ protected function parseValues(vec<token> $tokens): vec<Expression> {
             throw new SQLFakeParseException("Expected , between expressions in SET clause near {$token['value']}");
           }
           $expression_parser = new ExpressionParser($tokens, $pointer - 1);
-          $start = $pointer;
           list($pointer, $expression) = $expression_parser->buildWithPointer();
           $expressions[] = $expression;
           $needs_comma = true;

src/Parser/Lexer.php

@@ -62,6 +62,7 @@ private function groupComments(dict<int, string> $tokens): dict<int, string> {
 
         if (!$inline && ($token === "*/")) {
           /*HH_IGNORE_ERROR[4110] Indexing using comment, which may be null.*/
+          /*HH_IGNORE_ERROR[4324] Indexing using comment, which may be null.*/
           unset($tokens[$comment]);
           $comment = null;
         }

src/Parser/SQLParser.php

@@ -312,7 +312,6 @@ private static function isFunctionVersionOfOperator(
   public static function findMatchingParen(int $pointer, token_list $tokens): int {
     $paren_count = 0;
     $remaining_tokens = Vec\drop($tokens, $pointer);
-    $token_count = C\count($remaining_tokens);
     foreach ($remaining_tokens as $i => $token) {
       if ($token['type'] === TokenType::PAREN) {
         $paren_count++;

src/Parser/SelectParser.php

@@ -131,7 +131,6 @@ public function parse(): (int, SelectQuery) {
               $this->pointer++;
               $next = $this->tokens[$this->pointer] ?? null;
               $expressions = vec[];
-              $sort_directions = vec[];
               if ($next === null || $next['value'] !== 'BY') {
                 throw new SQLFakeParseException("Expected BY after GROUP");
               }

src/Parser/SetParser.php

@@ -37,7 +37,7 @@ public function parse(bool $skip_set = false): (int, vec<BinaryOperatorExpressio
             throw new SQLFakeParseException("Expected , between expressions in SET clause");
           }
           $expression_parser = new ExpressionParser($this->tokens, $this->pointer - 1);
-          $start = $this->pointer;
+          $this->pointer;
           list($this->pointer, $expression) = $expression_parser->buildWithPointer();
 
           // the only valid kind of expression in a SET is "foo = bar"

src/Query/FromClause.php

@@ -32,13 +32,12 @@ public function aliasRecentExpression(string $name): void {
   /**
    * The FROM clause of the query gets processed first, retrieving data from tables, executing subqueries, and handling joins
    * This is also where we build up the $columns list which is commonly used throughout the entire library to map column references to indexes in this dataset
+   * @reviewer, we don't build up the $columns, since the variable is unused...
    */
   public function process(AsyncMysqlConnection $conn, string $sql): dataset {
 
     $data = vec[];
-    $columns = vec[];
     $is_first_table = true;
-    $left_column_list = keyset[];
 
     foreach ($this->tables as $table) {
 

src/Query/JoinProcessor.php

@@ -149,9 +149,9 @@ protected static function buildNaturalJoinFilter(dataset $left_dataset, dataset
     if ($left === null || $right === null) {
       throw new SQLFakeParseException("Attempted NATURAL join with no data present");
     }
-    foreach ($left as $column => $val) {
+    foreach ($left as $column => $_) {
       $name = Str\split($column, '.') |> C\lastx($$);
-      foreach ($right as $col => $v) {
+      foreach ($right as $col => $_) {
         $colname = Str\split($col, '.') |> C\lastx($$);
         if ($colname === $name) {
           $filter = self::addJoinFilterExpression($filter, $column, $col);

src/Query/Query.php

@@ -42,7 +42,7 @@ protected function applyOrderBy(AsyncMysqlConnection $conn, dataset $data): data
     }
 
     // allow all column expressions to fall through to the full row
-    foreach ($order_by as $k => $rule) {
+    foreach ($order_by as $rule) {
       $expr = $rule['expression'];
       if ($expr is ColumnExpression) {
         $expr->allowFallthrough();
@@ -94,7 +94,7 @@ protected function applyOrderBy(AsyncMysqlConnection $conn, dataset $data): data
     // dicts maintain insert order. the keys will be inserted out of order but have to match the original
     // keys for updates/deletes to be able to delete the right rows
     $data = dict[];
-    foreach ($data_temp as $index => $item) {
+    foreach ($data_temp as $item) {
       $data[$item[0]] = $item[1];
     }
 
@@ -194,7 +194,7 @@ protected function applySet(
       foreach ($set_clauses as $clause) {
         $existing_value = $row[$clause['column']] ?? null;
         $expr = $clause['expression'];
-        $new_value = $clause['expression']->evaluate($update_row, $conn);
+        $new_value = $expr->evaluate($update_row, $conn);
 
         if ($new_value !== $existing_value) {
           $row[$clause['column']] = $new_value;