From b744ed9a19ce755dfa674013d8db92a273f6e356 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 13 May 2014 15:17:56 -0700
Subject: [PATCH] Be more strict with the type of `.arclint` properties.

Summary: Although this provides less context in terms of the error message (for example, `Parameter has invalid type. Expected type 'optional regex|list<regex>', got type 'list<string>'.`), I think that it is the right approach. I think that `PhutilTypeSpec::checkMap` should be improved such that additional context is provided in the exception message.

Test Plan: Ran `arc lint`. Modified `.arclint` to contain an invalid regex and ran `arc lint` again.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9089
---
 .../ArcanistConfigurationDrivenLintEngine.php | 71 ++++++++-----------
 1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
index 330e7397..709909e6 100644
--- a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
+++ b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
@@ -22,15 +22,21 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
 
     $linters = $this->loadAvailableLinters();
 
-    PhutilTypeSpec::checkMap(
-      $config,
-      array(
-        'exclude' => 'optional string | list<string>',
-        'linters' => 'map<string, map<string, wild>>',
-      ));
+    try {
+      PhutilTypeSpec::checkMap(
+        $config,
+        array(
+          'exclude' => 'optional regex | list<regex>',
+          'linters' => 'map<string, map<string, wild>>',
+        ));
+    } catch (PhutilTypeCheckException $ex) {
+      $message = pht(
+        'Error in parsing ".arclint" file: %s',
+        $ex->getMessage());
+      throw new PhutilProxyException($message, $ex);
+    }
 
     $global_exclude = (array)idx($config, 'exclude', array());
-    $this->validateRegexps($global_exclude);
 
     $built_linters = array();
     $all_paths = $this->getPaths();
@@ -63,13 +69,21 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
         $more = array();
       }
 
-      PhutilTypeSpec::checkMap(
-        $spec,
-        array(
-          'type' => 'string',
-          'include' => 'optional string | list<string>',
-          'exclude' => 'optional string | list<string>',
-        ) + $more);
+      try {
+        PhutilTypeSpec::checkMap(
+          $spec,
+          array(
+            'type' => 'string',
+            'include' => 'optional regex | list<regex>',
+            'exclude' => 'optional regex | list<regex>',
+          ) + $more);
+      } catch (PhutilTypeCheckException $ex) {
+        $message = pht(
+          'Error in parsing ".arclint" file, for linter "%s": %s',
+          $name,
+          $ex->getMessage());
+        throw new PhutilProxyException($message, $ex);
+      }
 
       foreach ($more as $key => $value) {
         if (array_key_exists($key, $spec)) {
@@ -90,9 +104,6 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
       $include = (array)idx($spec, 'include', array());
       $exclude = (array)idx($spec, 'exclude', array());
 
-      $this->validateRegexps($include, $name, 'include');
-      $this->validateRegexps($exclude, $name, 'exclude');
-
       $console = PhutilConsole::getConsole();
       $console->writeLog("Examining paths for linter \"%s\".\n", $name);
       $paths = $this->matchPaths(
@@ -219,30 +230,4 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
     return $match;
   }
 
-  private function validateRegexps(
-    array $regexps,
-    $linter = null,
-    $config = null) {
-
-    foreach ($regexps as $regexp) {
-      $ok = @preg_match($regexp, '');
-      if ($ok === false) {
-        if ($linter) {
-          throw new Exception(
-            pht(
-              'Regular expression "%s" (in "%s" configuration for linter '.
-              '"%s") is not a valid regular expression.',
-              $regexp,
-              $config,
-              $linter));
-        } else {
-          throw new Exception(
-            pht(
-              'Regular expression "%s" is not a valid regular expression.',
-              $regexp));
-        }
-      }
-    }
-  }
-
 }