From ff466b79923d0ea2bcd59f16370fce8df569901e Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 08:42:39 -0500 Subject: [PATCH 1/8] Change config API --- psalm.xml | 41 +++++++++++----------- src/Psalm/Config.php | 18 +++++----- src/Psalm/Config/FileFilter.php | 60 ++++++++++++++++----------------- tests/ArrayAccessTest.php | 4 +-- tests/ArrayAssignmentTest.php | 2 +- tests/FunctionCallTest.php | 2 +- tests/IssueSuppressionTest.php | 8 ++--- tests/MethodCallTest.php | 2 +- tests/Php70Test.php | 4 +-- tests/PropertyTypeTest.php | 8 ++--- 10 files changed, 75 insertions(+), 74 deletions(-) diff --git a/psalm.xml b/psalm.xml index adca273c09a..9dd056e33c9 100644 --- a/psalm.xml +++ b/psalm.xml @@ -6,66 +6,67 @@ totallyTyped="true" strictBinaryOperands="false" > - + - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 5147a4f8b66..ea2f064b823 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -88,7 +88,7 @@ class Config /** * @var FileFilter|null */ - protected $inspect_files; + protected $project_files; /** * The base directory of this config file @@ -228,8 +228,8 @@ public static function loadFromXML($file_name) $config->strict_binary_operands = $attribute_text === 'true' || $attribute_text === '1'; } - if (isset($config_xml->inspectFiles)) { - $config->inspect_files = FileFilter::loadFromXML($config_xml->inspectFiles, true); + if (isset($config_xml->projectFiles)) { + $config->project_files = FileFilter::loadFromXML($config_xml->projectFiles, true); } if (isset($config_xml->fileExtensions)) { @@ -289,12 +289,12 @@ public static function loadFromXML($file_name) $config->custom_error_levels[$key] = $error_level; } - if (isset($issue_handler->excludeFiles)) { - $config->issue_handlers[$key] = FileFilter::loadFromXML($issue_handler->excludeFiles, false); + if (isset($issue_handler->ignoreFiles)) { + $config->issue_handlers[$key] = FileFilter::loadFromXML($issue_handler->ignoreFiles, false); } - if (isset($issue_handler->includeFiles)) { - $config->issue_handlers[$key] = FileFilter::loadFromXML($issue_handler->includeFiles, true); + if (isset($issue_handler->onlyFiles)) { + $config->issue_handlers[$key] = FileFilter::loadFromXML($issue_handler->onlyFiles, true); } } } @@ -439,11 +439,11 @@ public function getReportingLevel($issue_type) */ public function getIncludeDirs() { - if (!$this->inspect_files) { + if (!$this->project_files) { return []; } - return $this->inspect_files->getIncludeDirs(); + return $this->project_files->getIncludeDirs(); } /** diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php index bfcd0cf354d..059a912e744 100644 --- a/src/Psalm/Config/FileFilter.php +++ b/src/Psalm/Config/FileFilter.php @@ -8,32 +8,32 @@ class FileFilter /** * @var array */ - protected $include_dirs = []; + protected $only_dirs = []; /** * @var array */ - protected $exclude_dirs = []; + protected $ignore_dirs = []; /** * @var array */ - protected $include_files = []; + protected $only_files = []; /** * @var array */ - protected $include_files_lowercase = []; + protected $only_files_lowercase = []; /** * @var array */ - protected $exclude_files = []; + protected $ignore_files = []; /** * @var array */ - protected $exclude_files_lowercase = []; + protected $ignore_files_lowercase = []; /** * @var array @@ -77,28 +77,28 @@ public static function loadFromXML(SimpleXMLElement $e, $inclusive) if ($e->directory) { /** @var \SimpleXMLElement $directory */ foreach ($e->directory as $directory) { - $filter->addIncludeDirectory((string)$directory['name']); + $filter->addOnlyDirectory((string)$directory['name']); } } if ($e->file) { /** @var \SimpleXMLElement $file */ foreach ($e->file as $file) { - $filter->addIncludeFile((string)$file['name']); + $filter->addOnlyFile((string)$file['name']); } } } else { if ($e->directory) { /** @var \SimpleXMLElement $directory */ foreach ($e->directory as $directory) { - $filter->addExcludeDirectory((string)$directory['name']); + $filter->addIgnoreDirectory((string)$directory['name']); } } if ($e->file) { /** @var \SimpleXMLElement $file */ foreach ($e->file as $file) { - $filter->addExcludeFile((string)$file['name']); + $filter->addIgnoreFile((string)$file['name']); } } } @@ -123,7 +123,7 @@ protected static function slashify($str) public function allows($file_name, $case_sensitive = false) { if ($this->inclusive) { - foreach ($this->include_dirs as $include_dir) { + foreach ($this->only_dirs as $include_dir) { if ($case_sensitive) { if (strpos($file_name, $include_dir) === 0) { return true; @@ -136,11 +136,11 @@ public function allows($file_name, $case_sensitive = false) } if ($case_sensitive) { - if (in_array($file_name, $this->include_files)) { + if (in_array($file_name, $this->only_files)) { return true; } } else { - if (in_array(strtolower($file_name), $this->include_files_lowercase)) { + if (in_array(strtolower($file_name), $this->only_files_lowercase)) { return true; } } @@ -149,7 +149,7 @@ public function allows($file_name, $case_sensitive = false) } // exclusive - foreach ($this->exclude_dirs as $exclude_dir) { + foreach ($this->ignore_dirs as $exclude_dir) { if ($case_sensitive) { if (strpos($file_name, $exclude_dir) === 0) { return false; @@ -162,11 +162,11 @@ public function allows($file_name, $case_sensitive = false) } if ($case_sensitive) { - if (in_array($file_name, $this->exclude_files)) { + if (in_array($file_name, $this->ignore_files)) { return false; } } else { - if (in_array(strtolower($file_name), $this->exclude_files_lowercase)) { + if (in_array(strtolower($file_name), $this->ignore_files_lowercase)) { return false; } } @@ -179,7 +179,7 @@ public function allows($file_name, $case_sensitive = false) */ public function getIncludeDirs() { - return $this->include_dirs; + return $this->only_dirs; } /** @@ -187,7 +187,7 @@ public function getIncludeDirs() */ public function getExcludeDirs() { - return $this->exclude_dirs; + return $this->ignore_dirs; } /** @@ -195,7 +195,7 @@ public function getExcludeDirs() */ public function getIncludeFiles() { - return $this->include_files; + return $this->only_files; } /** @@ -203,52 +203,52 @@ public function getIncludeFiles() */ public function getExcludeFiles() { - return $this->exclude_files; + return $this->ignore_files; } /** * @param string $file_name * @return void */ - public function addExcludeFile($file_name) + public function addIgnoreFile($file_name) { if ($this->inclusive !== false) { throw new \UnexpectedValueException('Cannot add exclude file when filter is not exclusive'); } - $this->exclude_files[] = $file_name; - $this->exclude_files_lowercase[] = strtolower($file_name); + $this->ignore_files[] = $file_name; + $this->ignore_files_lowercase[] = strtolower($file_name); } /** * @param string $file_name * @return void */ - public function addIncludeFile($file_name) + public function addOnlyFile($file_name) { if ($this->inclusive !== true) { throw new \UnexpectedValueException('Cannot add include file when filter is not inclusive'); } - $this->include_files[] = $file_name; - $this->include_files_lowercase[] = strtolower($file_name); + $this->only_files[] = $file_name; + $this->only_files_lowercase[] = strtolower($file_name); } /** * @param string $dir_name * @return void */ - public function addExcludeDirectory($dir_name) + public function addIgnoreDirectory($dir_name) { - $this->exclude_dirs[] = self::slashify($dir_name); + $this->ignore_dirs[] = self::slashify($dir_name); } /** * @param string $dir_name * @return void */ - public function addIncludeDirectory($dir_name) + public function addOnlyDirectory($dir_name) { - $this->include_dirs[] = self::slashify($dir_name); + $this->only_dirs[] = self::slashify($dir_name); } } diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php index fec890eb0b7..46352648dc4 100644 --- a/tests/ArrayAccessTest.php +++ b/tests/ArrayAccessTest.php @@ -121,7 +121,7 @@ public function testInvalidArrayAccess() public function testMixedArrayAccess() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $context = new Context('somefile.php'); @@ -142,7 +142,7 @@ public function testMixedArrayAccess() public function testMixedArrayOffset() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $context = new Context('somefile.php'); diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index 9e8bdc6b2c4..a3c002cdb97 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -662,7 +662,7 @@ public function testInvalidArrayAccess() public function testMixedStringOffsetAssignment() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $context = new Context('somefile.php'); diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index 3a3139516fb..7388abe5743 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -46,7 +46,7 @@ function foo(int $a) : void {} public function testMixedArgument() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $stmts = self::$parser->parse('addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); $stmts = self::$parser->parse('addIncludeFile('somefile.php'); + $filter->addOnlyFile('somefile.php'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); $stmts = self::$parser->parse('addExcludeDirectory('src'); + $filter->addIgnoreDirectory('src'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); $stmts = self::$parser->parse('addIncludeDirectory('src2'); + $filter->addOnlyDirectory('src2'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); (new FileChecker( diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index 4f6f6c7f982..2f677d7a2c7 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -64,7 +64,7 @@ public static function bar() : void {} public function testMixedMethodCall() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); diff --git a/tests/Php70Test.php b/tests/Php70Test.php index 36db48a1e02..7e4d3eba206 100644 --- a/tests/Php70Test.php +++ b/tests/Php70Test.php @@ -74,7 +74,7 @@ public static function indexof(string $haystack, string $needle) : int public function testNullCoalesce() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $stmts = self::$parser->parse('addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $stmts = self::$parser->parse('addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); @@ -134,7 +134,7 @@ class A { public function foo() : void { echo $this->foo; } - } + } '); $file_checker = new FileChecker('somefile.php', $stmts); @@ -291,7 +291,7 @@ class B { public function testMixedPropertyFetch() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); @@ -319,7 +319,7 @@ class Foo { public function testMixedPropertyAssignment() { $filter = new Config\FileFilter(false); - $filter->addExcludeFile('somefile.php'); + $filter->addIgnoreFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); From 749735a3ecb8154f40054354e88c51b4560013b9 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 10:24:10 -0500 Subject: [PATCH 2/8] Update config file API still further --- psalm.xml | 1 - src/Psalm/Checker/ProjectChecker.php | 10 +- src/Psalm/Config.php | 44 +++++---- src/Psalm/Config/FileFilter.php | 137 ++++++--------------------- tests/ArrayAccessTest.php | 4 +- tests/ArrayAssignmentTest.php | 2 +- tests/ConfigTest.php | 30 ++++++ tests/FunctionCallTest.php | 2 +- tests/IssueSuppressionTest.php | 8 +- tests/MethodCallTest.php | 2 +- tests/Php70Test.php | 4 +- tests/PropertyTypeTest.php | 6 +- 12 files changed, 106 insertions(+), 144 deletions(-) create mode 100644 tests/ConfigTest.php diff --git a/psalm.xml b/psalm.xml index 9dd056e33c9..26ed1955503 100644 --- a/psalm.xml +++ b/psalm.xml @@ -11,7 +11,6 @@ - diff --git a/src/Psalm/Checker/ProjectChecker.php b/src/Psalm/Checker/ProjectChecker.php index 6ca0aed48e4..823be4e0b6c 100644 --- a/src/Psalm/Checker/ProjectChecker.php +++ b/src/Psalm/Checker/ProjectChecker.php @@ -101,7 +101,7 @@ public function check($debug = false, $is_diff = false, $update_docblocks = fals $deleted_files = FileChecker::getDeletedReferencedFiles(); $diff_files = $deleted_files; - foreach ($this->config->getIncludeDirs() as $dir_name) { + foreach ($this->config->getProjectDirectories() as $dir_name) { $diff_files = array_merge($diff_files, self::getDiffFilesInDir($dir_name, $this->config)); } } @@ -109,7 +109,7 @@ public function check($debug = false, $is_diff = false, $update_docblocks = fals $files_checked = []; if ($diff_files === null || $deleted_files === null || count($diff_files) > 200) { - foreach ($this->config->getIncludeDirs() as $dir_name) { + foreach ($this->config->getProjectDirectories() as $dir_name) { $this->checkDirWithConfig($dir_name, $this->config, $debug, $update_docblocks); } } else { @@ -211,7 +211,7 @@ protected function getAllFiles(Config $config) $file_extensions = $config->getFileExtensions(); $file_names = []; - foreach ($config->getIncludeDirs() as $dir_name) { + foreach ($config->getProjectDirectories() as $dir_name) { /** @var RecursiveDirectoryIterator */ $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir_name)); $iterator->rewind(); @@ -371,7 +371,7 @@ protected static function getConfigForPath($path) $maybe_path = $dir_path . Config::DEFAULT_FILE_NAME; if (file_exists($maybe_path)) { - $config = Config::loadFromXML($maybe_path); + $config = Config::loadFromXMLFile($maybe_path); if ($config->autoloader) { require_once($dir_path . $config->autoloader); @@ -405,7 +405,7 @@ public function setConfigXML($path_to_config) $dir_path = dirname($path_to_config) . '/'; - $this->config = Config::loadFromXML($path_to_config); + $this->config = Config::loadFromXMLFile($path_to_config); if ($this->config->autoloader) { require_once($dir_path . $this->config->autoloader); diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index ea2f064b823..5d5e014c8cb 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -154,7 +154,25 @@ protected function __construct() /** * Creates a new config object from the file * - * @param string $file_name + * @param string $file_path + * @return self + */ + public static function loadFromXMLFile($file_path) + { + $file_contents = file_get_contents($file_path); + + if ($file_contents === false) { + throw new \InvalidArgumentException('Cannot open ' . $file_path); + } + + return self::loadFromXML($file_path, $file_contents); + } + + /** + * Creates a new config object from an XML string + * + * @param string $file_path + * @param string $file_contents * @return self * @psalm-suppress MixedArgument * @psalm-suppress MixedPropertyFetch @@ -162,17 +180,11 @@ protected function __construct() * @psalm-suppress MixedAssignment * @psalm-suppress MixedOperand */ - public static function loadFromXML($file_name) + public static function loadFromXML($file_path, $file_contents) { - $file_contents = file_get_contents($file_name); - - if ($file_contents === false) { - throw new \InvalidArgumentException('Cannot open ' . $file_name); - } - $config = new self(); - $config->base_dir = dirname($file_name) . '/'; + $config->base_dir = dirname($file_path) . '/'; $config_xml = new SimpleXMLElement($file_contents); @@ -229,7 +241,7 @@ public static function loadFromXML($file_name) } if (isset($config_xml->projectFiles)) { - $config->project_files = FileFilter::loadFromXML($config_xml->projectFiles, true); + $config->project_files = FileFilter::loadFromXMLElement($config_xml->projectFiles, true); } if (isset($config_xml->fileExtensions)) { @@ -290,11 +302,11 @@ public static function loadFromXML($file_name) } if (isset($issue_handler->ignoreFiles)) { - $config->issue_handlers[$key] = FileFilter::loadFromXML($issue_handler->ignoreFiles, false); + $config->issue_handlers[$key] = FileFilter::loadFromXMLElement($issue_handler->ignoreFiles, false); } if (isset($issue_handler->onlyFiles)) { - $config->issue_handlers[$key] = FileFilter::loadFromXML($issue_handler->onlyFiles, true); + $config->issue_handlers[$key] = FileFilter::loadFromXMLElement($issue_handler->onlyFiles, true); } } } @@ -393,7 +405,7 @@ public function excludeIssueInFile($issue_type, $file_name) $file_name = $this->shortenFileName($file_name); - if ($this->getIncludeDirs() && $this->hide_external_errors) { + if ($this->getProjectDirectories() && $this->hide_external_errors) { if (!$this->isInProjectDirs($file_name)) { return true; } @@ -412,7 +424,7 @@ public function excludeIssueInFile($issue_type, $file_name) */ public function isInProjectDirs($file_name) { - foreach ($this->getIncludeDirs() as $dir_name) { + foreach ($this->getProjectDirectories() as $dir_name) { if (preg_match('/^' . preg_quote($dir_name, '/') . '/', $file_name)) { return true; } @@ -437,13 +449,13 @@ public function getReportingLevel($issue_type) /** * @return array */ - public function getIncludeDirs() + public function getProjectDirectories() { if (!$this->project_files) { return []; } - return $this->project_files->getIncludeDirs(); + return $this->project_files->getDirectories(); } /** diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php index 059a912e744..082cafa93e3 100644 --- a/src/Psalm/Config/FileFilter.php +++ b/src/Psalm/Config/FileFilter.php @@ -8,42 +8,22 @@ class FileFilter /** * @var array */ - protected $only_dirs = []; + protected $directories = []; /** * @var array */ - protected $ignore_dirs = []; + protected $files = []; /** * @var array */ - protected $only_files = []; + protected $files_lowercase = []; /** * @var array */ - protected $only_files_lowercase = []; - - /** - * @var array - */ - protected $ignore_files = []; - - /** - * @var array - */ - protected $ignore_files_lowercase = []; - - /** - * @var array - */ - protected $include_patterns = []; - - /** - * @var array - */ - protected $exclude_patterns = []; + protected $patterns = []; /** * @var bool @@ -69,37 +49,21 @@ public function __construct($inclusive) * @param bool $inclusive * @return self */ - public static function loadFromXML(SimpleXMLElement $e, $inclusive) + public static function loadFromXMLElement(SimpleXMLElement $e, $inclusive) { $filter = new self($inclusive); - if ($inclusive) { - if ($e->directory) { - /** @var \SimpleXMLElement $directory */ - foreach ($e->directory as $directory) { - $filter->addOnlyDirectory((string)$directory['name']); - } - } - - if ($e->file) { - /** @var \SimpleXMLElement $file */ - foreach ($e->file as $file) { - $filter->addOnlyFile((string)$file['name']); - } - } - } else { - if ($e->directory) { - /** @var \SimpleXMLElement $directory */ - foreach ($e->directory as $directory) { - $filter->addIgnoreDirectory((string)$directory['name']); - } + if ($e->directory) { + /** @var \SimpleXMLElement $directory */ + foreach ($e->directory as $directory) { + $filter->addDirectory((string)$directory['name']); } + } - if ($e->file) { - /** @var \SimpleXMLElement $file */ - foreach ($e->file as $file) { - $filter->addIgnoreFile((string)$file['name']); - } + if ($e->file) { + /** @var \SimpleXMLElement $file */ + foreach ($e->file as $file) { + $filter->addFile((string)$file['name']); } } @@ -123,7 +87,7 @@ protected static function slashify($str) public function allows($file_name, $case_sensitive = false) { if ($this->inclusive) { - foreach ($this->only_dirs as $include_dir) { + foreach ($this->directories as $include_dir) { if ($case_sensitive) { if (strpos($file_name, $include_dir) === 0) { return true; @@ -136,11 +100,11 @@ public function allows($file_name, $case_sensitive = false) } if ($case_sensitive) { - if (in_array($file_name, $this->only_files)) { + if (in_array($file_name, $this->files)) { return true; } } else { - if (in_array(strtolower($file_name), $this->only_files_lowercase)) { + if (in_array(strtolower($file_name), $this->files_lowercase)) { return true; } } @@ -149,7 +113,7 @@ public function allows($file_name, $case_sensitive = false) } // exclusive - foreach ($this->ignore_dirs as $exclude_dir) { + foreach ($this->directories as $exclude_dir) { if ($case_sensitive) { if (strpos($file_name, $exclude_dir) === 0) { return false; @@ -162,11 +126,11 @@ public function allows($file_name, $case_sensitive = false) } if ($case_sensitive) { - if (in_array($file_name, $this->ignore_files)) { + if (in_array($file_name, $this->files)) { return false; } } else { - if (in_array(strtolower($file_name), $this->ignore_files_lowercase)) { + if (in_array(strtolower($file_name), $this->files_lowercase)) { return false; } } @@ -177,78 +141,35 @@ public function allows($file_name, $case_sensitive = false) /** * @return array */ - public function getIncludeDirs() + public function getDirectories() { - return $this->only_dirs; + return $this->directories; } /** * @return array */ - public function getExcludeDirs() + public function getFiles() { - return $this->ignore_dirs; - } - - /** - * @return array - */ - public function getIncludeFiles() - { - return $this->only_files; - } - - /** - * @return array - */ - public function getExcludeFiles() - { - return $this->ignore_files; + return $this->files; } /** * @param string $file_name * @return void */ - public function addIgnoreFile($file_name) - { - if ($this->inclusive !== false) { - throw new \UnexpectedValueException('Cannot add exclude file when filter is not exclusive'); - } - - $this->ignore_files[] = $file_name; - $this->ignore_files_lowercase[] = strtolower($file_name); - } - - /** - * @param string $file_name - * @return void - */ - public function addOnlyFile($file_name) - { - if ($this->inclusive !== true) { - throw new \UnexpectedValueException('Cannot add include file when filter is not inclusive'); - } - - $this->only_files[] = $file_name; - $this->only_files_lowercase[] = strtolower($file_name); - } - - /** - * @param string $dir_name - * @return void - */ - public function addIgnoreDirectory($dir_name) + public function addFile($file_name) { - $this->ignore_dirs[] = self::slashify($dir_name); + $this->files[] = $file_name; + $this->files_lowercase[] = strtolower($file_name); } /** * @param string $dir_name * @return void */ - public function addOnlyDirectory($dir_name) + public function addDirectory($dir_name) { - $this->only_dirs[] = self::slashify($dir_name); + $this->directories[] = self::slashify($dir_name); } } diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php index 46352648dc4..75bbcffaa08 100644 --- a/tests/ArrayAccessTest.php +++ b/tests/ArrayAccessTest.php @@ -121,7 +121,7 @@ public function testInvalidArrayAccess() public function testMixedArrayAccess() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $context = new Context('somefile.php'); @@ -142,7 +142,7 @@ public function testMixedArrayAccess() public function testMixedArrayOffset() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $context = new Context('somefile.php'); diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php index a3c002cdb97..15ff386e94a 100644 --- a/tests/ArrayAssignmentTest.php +++ b/tests/ArrayAssignmentTest.php @@ -662,7 +662,7 @@ public function testInvalidArrayAccess() public function testMixedStringOffsetAssignment() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $context = new Context('somefile.php'); diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php new file mode 100644 index 00000000000..5ca3cf5f529 --- /dev/null +++ b/tests/ConfigTest.php @@ -0,0 +1,30 @@ + + + + + +'); + + $this->assertTrue($config->isInProjectDirs('src/main.php')); + $this->assertFalse($config->isInProjectDirs('main.php')); + } +} diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index 7388abe5743..98cea804941 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -46,7 +46,7 @@ function foo(int $a) : void {} public function testMixedArgument() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $stmts = self::$parser->parse('addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); $stmts = self::$parser->parse('addOnlyFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); $stmts = self::$parser->parse('addIgnoreDirectory('src'); + $filter->addDirectory('src'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); $stmts = self::$parser->parse('addOnlyDirectory('src2'); + $filter->addDirectory('src2'); Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); (new FileChecker( diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index 2f677d7a2c7..b4f9e19f46a 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -64,7 +64,7 @@ public static function bar() : void {} public function testMixedMethodCall() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); diff --git a/tests/Php70Test.php b/tests/Php70Test.php index 7e4d3eba206..3567fdc51ce 100644 --- a/tests/Php70Test.php +++ b/tests/Php70Test.php @@ -74,7 +74,7 @@ public static function indexof(string $haystack, string $needle) : int public function testNullCoalesce() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $stmts = self::$parser->parse('addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); $stmts = self::$parser->parse('addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); @@ -291,7 +291,7 @@ class B { public function testMixedPropertyFetch() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); @@ -319,7 +319,7 @@ class Foo { public function testMixedPropertyAssignment() { $filter = new Config\FileFilter(false); - $filter->addIgnoreFile('somefile.php'); + $filter->addFile('somefile.php'); Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); Config::getInstance()->setIssueHandler('MixedAssignment', $filter); From b79ce904d2e8c02b3325438ea11c7c661085bd97 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 18:33:03 -0500 Subject: [PATCH 3/8] Fix #23 - allow exclusion in projectFiles --- src/Psalm/Config.php | 10 ++-------- src/Psalm/Config/FileFilter.php | 20 ++++++++++++++++++++ tests/ConfigTest.php | 17 +++++++++++++++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 5d5e014c8cb..7e3f7a549c3 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -405,7 +405,7 @@ public function excludeIssueInFile($issue_type, $file_name) $file_name = $this->shortenFileName($file_name); - if ($this->getProjectDirectories() && $this->hide_external_errors) { + if ($this->project_files && $this->hide_external_errors) { if (!$this->isInProjectDirs($file_name)) { return true; } @@ -424,13 +424,7 @@ public function excludeIssueInFile($issue_type, $file_name) */ public function isInProjectDirs($file_name) { - foreach ($this->getProjectDirectories() as $dir_name) { - if (preg_match('/^' . preg_quote($dir_name, '/') . '/', $file_name)) { - return true; - } - } - - return false; + return $this->project_files && $this->project_files->allows($file_name); } /** diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php index 082cafa93e3..d5f3d7ac78b 100644 --- a/src/Psalm/Config/FileFilter.php +++ b/src/Psalm/Config/FileFilter.php @@ -20,6 +20,11 @@ class FileFilter */ protected $files_lowercase = []; + /** + * @var FileFilter|null + */ + protected $file_filter = null; + /** * @var array */ @@ -67,6 +72,15 @@ public static function loadFromXMLElement(SimpleXMLElement $e, $inclusive) } } + if (isset($e->ignoreFiles)) { + if (!$inclusive) { + throw new \Psalm\Exception\ConfigException('Cannot nest ignoreFiles inside itself'); + } + + /** @var \SimpleXMLElement $e->ignoreFiles */ + $filter->file_filter = self::loadFromXMLElement($e->ignoreFiles, false); + } + return $filter; } @@ -87,6 +101,12 @@ protected static function slashify($str) public function allows($file_name, $case_sensitive = false) { if ($this->inclusive) { + if ($this->file_filter) { + if (!$this->file_filter->allows($file_name, $case_sensitive)) { + return false; + } + } + foreach ($this->directories as $include_dir) { if ($case_sensitive) { if (strpos($file_name, $include_dir) === 0) { diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index 5ca3cf5f529..7b1c8e0aff2 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -27,4 +27,21 @@ public function testBarebonesConfig() $this->assertTrue($config->isInProjectDirs('src/main.php')); $this->assertFalse($config->isInProjectDirs('main.php')); } + + public function testIgnoreProjectDirectory() + { + $config = Config::loadFromXML('psalm.xml', ' + + + + + + + +'); + + $this->assertTrue($config->isInProjectDirs('src/main.php')); + $this->assertFalse($config->isInProjectDirs('src/ignoreme/main.php')); + $this->assertFalse($config->isInProjectDirs('main.php')); + } } From 685eaeb4fee51c76dca3c02aeb9b3f2f267128f2 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 20:07:42 -0500 Subject: [PATCH 4/8] Reimagine XML schema, fixes #21 --- README.md | 4 +- examples/psalm.default.xml | 8 +-- psalm.xml | 40 ++++++------- src/Psalm/Config.php | 64 +++++++-------------- src/Psalm/Config/ErrorLevelFileFilter.php | 44 ++++++++++++++ src/Psalm/Config/FileFilter.php | 30 ++-------- src/Psalm/Config/IssueHandler.php | 69 ++++++++++++++++++++++ src/Psalm/Config/ProjectFileFilter.php | 53 +++++++++++++++++ src/Psalm/IssueBuffer.php | 2 +- tests/ArrayAccessTest.php | 8 +-- tests/ArrayAssignmentTest.php | 4 +- tests/ConfigTest.php | 44 ++++++++++++++ tests/FunctionCallTest.php | 4 +- tests/IssueSuppressionTest.php | 70 +---------------------- tests/MethodCallTest.php | 6 +- tests/Php70Test.php | 8 +-- tests/PropertyTypeTest.php | 18 ++---- tests/ScopeTest.php | 5 -- 18 files changed, 281 insertions(+), 200 deletions(-) create mode 100644 src/Psalm/Config/ErrorLevelFileFilter.php create mode 100644 src/Psalm/Config/IssueHandler.php create mode 100644 src/Psalm/Config/ProjectFileFilter.php diff --git a/README.md b/README.md index 72d239b53dd..e00a2ca6b7e 100644 --- a/README.md +++ b/README.md @@ -27,9 +27,9 @@ cat > psalm.xml << EOF stopOnFirstError="false" useDocblockTypes="true" > - + - + EOF ``` diff --git a/examples/psalm.default.xml b/examples/psalm.default.xml index 01bfdac2eb8..1c182a7afbf 100644 --- a/examples/psalm.default.xml +++ b/examples/psalm.default.xml @@ -5,15 +5,15 @@ useDocblockTypes="true" totallyTyped="false" > - + - + - + - + diff --git a/psalm.xml b/psalm.xml index 26ed1955503..576c3101a10 100644 --- a/psalm.xml +++ b/psalm.xml @@ -11,61 +11,61 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 7e3f7a549c3..6ca456155c4 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -2,7 +2,8 @@ namespace Psalm; use Psalm\Checker\FileChecker; -use Psalm\Config\FileFilter; +use Psalm\Config\IssueHandler; +use Psalm\Config\ProjectFileFilter; use Psalm\Exception\ConfigException; use SimpleXMLElement; @@ -86,7 +87,7 @@ class Config public $autoloader; /** - * @var FileFilter|null + * @var ProjectFileFilter|null */ protected $project_files; @@ -108,15 +109,10 @@ class Config protected $filetype_handlers = []; /** - * @var array + * @var array */ protected $issue_handlers = []; - /** - * @var array - */ - protected $custom_error_levels = []; - /** * @var array */ @@ -241,7 +237,7 @@ public static function loadFromXML($file_path, $file_contents) } if (isset($config_xml->projectFiles)) { - $config->project_files = FileFilter::loadFromXMLElement($config_xml->projectFiles, true); + $config->project_files = ProjectFileFilter::loadFromXMLElement($config_xml->projectFiles, true); } if (isset($config_xml->fileExtensions)) { @@ -288,26 +284,12 @@ public static function loadFromXML($file_path, $file_contents) } } - if (isset($config_xml->issueHandler)) { + if (isset($config_xml->issueHandlers)) { /** @var \SimpleXMLElement $issue_handler */ - foreach ($config_xml->issueHandler->children() as $key => $issue_handler) { - if (isset($issue_handler['errorLevel'])) { - $error_level = (string) $issue_handler['errorLevel']; - - if (!in_array($error_level, self::$ERROR_LEVELS)) { - throw new \InvalidArgumentException('Error level ' . $error_level . ' could not be recognised'); - } - - $config->custom_error_levels[$key] = $error_level; - } - - if (isset($issue_handler->ignoreFiles)) { - $config->issue_handlers[$key] = FileFilter::loadFromXMLElement($issue_handler->ignoreFiles, false); - } - - if (isset($issue_handler->onlyFiles)) { - $config->issue_handlers[$key] = FileFilter::loadFromXMLElement($issue_handler->onlyFiles, true); - } + foreach ($config_xml->issueHandlers->children() as $key => $issue_handler) { + $config->issue_handlers[$key] = IssueHandler::loadFromXMLElement( + $issue_handler + ); } } @@ -333,7 +315,8 @@ public static function getInstance() */ public function setCustomErrorLevel($issue_key, $error_level) { - $this->custom_error_levels[$issue_key] = $error_level; + $this->issue_handlers[$issue_key] = new IssueHandler(); + $this->issue_handlers[$issue_key]->setErrorLevel($error_level); } /** @@ -399,10 +382,6 @@ public function excludeIssueInFile($issue_type, $file_name) return true; } - if ($this->getReportingLevel($issue_type) === self::REPORT_SUPPRESS) { - return true; - } - $file_name = $this->shortenFileName($file_name); if ($this->project_files && $this->hide_external_errors) { @@ -411,11 +390,11 @@ public function excludeIssueInFile($issue_type, $file_name) } } - if (!isset($this->issue_handlers[$issue_type])) { - return false; + if ($this->getReportingLevelForFile($issue_type, $file_name) === self::REPORT_SUPPRESS) { + return true; } - return !$this->issue_handlers[$issue_type]->allows($file_name); + return false; } /** @@ -429,12 +408,13 @@ public function isInProjectDirs($file_name) /** * @param string $issue_type + * @param string $file_name * @return string */ - public function getReportingLevel($issue_type) + public function getReportingLevelForFile($issue_type, $file_name) { - if (isset($this->custom_error_levels[$issue_type])) { - return $this->custom_error_levels[$issue_type]; + if (isset($this->issue_handlers[$issue_type])) { + return $this->issue_handlers[$issue_type]->getReportingLevelForFile($file_name); } return self::REPORT_ERROR; @@ -502,12 +482,12 @@ public function getPlugins() /** * @param string $issue_name - * @param FileFilter|null $filter + * @param IssueHandler|null $handler * @return void */ - public function setIssueHandler($issue_name, FileFilter $filter = null) + public function setIssueHandler($issue_name, IssueHandler $handler = null) { - $this->issue_handlers[$issue_name] = $filter; + $this->issue_handlers[$issue_name] = $handler; } /** diff --git a/src/Psalm/Config/ErrorLevelFileFilter.php b/src/Psalm/Config/ErrorLevelFileFilter.php new file mode 100644 index 00000000000..96014ca7577 --- /dev/null +++ b/src/Psalm/Config/ErrorLevelFileFilter.php @@ -0,0 +1,44 @@ +error_level = (string) $e['type']; + + if (!in_array($filter->error_level, \Psalm\Config::$ERROR_LEVELS)) { + throw new \Psalm\Exception\ConfigException('Unexepected error level ' . $filter->error_level); + } + } else { + throw new \Psalm\Exception\ConfigException(' element expects a level'); + } + + return $filter; + } + + /** + * @return string + */ + public function getErrorLevel() + { + return $this->error_level; + } +} diff --git a/src/Psalm/Config/FileFilter.php b/src/Psalm/Config/FileFilter.php index d5f3d7ac78b..2d4a7bc2928 100644 --- a/src/Psalm/Config/FileFilter.php +++ b/src/Psalm/Config/FileFilter.php @@ -20,11 +20,6 @@ class FileFilter */ protected $files_lowercase = []; - /** - * @var FileFilter|null - */ - protected $file_filter = null; - /** * @var array */ @@ -52,11 +47,13 @@ public function __construct($inclusive) /** * @param SimpleXMLElement $e * @param bool $inclusive - * @return self + * @return static */ - public static function loadFromXMLElement(SimpleXMLElement $e, $inclusive) - { - $filter = new self($inclusive); + public static function loadFromXMLElement( + SimpleXMLElement $e, + $inclusive + ) { + $filter = new static($inclusive); if ($e->directory) { /** @var \SimpleXMLElement $directory */ @@ -72,15 +69,6 @@ public static function loadFromXMLElement(SimpleXMLElement $e, $inclusive) } } - if (isset($e->ignoreFiles)) { - if (!$inclusive) { - throw new \Psalm\Exception\ConfigException('Cannot nest ignoreFiles inside itself'); - } - - /** @var \SimpleXMLElement $e->ignoreFiles */ - $filter->file_filter = self::loadFromXMLElement($e->ignoreFiles, false); - } - return $filter; } @@ -101,12 +89,6 @@ protected static function slashify($str) public function allows($file_name, $case_sensitive = false) { if ($this->inclusive) { - if ($this->file_filter) { - if (!$this->file_filter->allows($file_name, $case_sensitive)) { - return false; - } - } - foreach ($this->directories as $include_dir) { if ($case_sensitive) { if (strpos($file_name, $include_dir) === 0) { diff --git a/src/Psalm/Config/IssueHandler.php b/src/Psalm/Config/IssueHandler.php new file mode 100644 index 00000000000..9de1fdf7dea --- /dev/null +++ b/src/Psalm/Config/IssueHandler.php @@ -0,0 +1,69 @@ + + */ + protected $custom_levels = []; + + /** + * @param SimpleXMLElement $e + * @return self + */ + public static function loadFromXMLElement(SimpleXMLElement $e) + { + $handler = new self(); + + if (isset($e['errorLevel'])) { + $handler->error_level = (string) $e['errorLevel']; + + if (!in_array($handler->error_level, \Psalm\Config::$ERROR_LEVELS)) { + throw new \Psalm\Exception\ConfigException('Unexepected error level ' . $handler->error_level); + } + } + + /** @var \SimpleXMLElement $error_level */ + foreach ($e->errorLevel as $error_level) { + $handler->custom_levels[] = ErrorLevelFileFilter::loadFromXMLElement($error_level, true); + } + + return $handler; + } + + /** + * @param string $error_level + * @return void + */ + public function setErrorLevel($error_level) + { + if (!in_array($error_level, \Psalm\Config::$ERROR_LEVELS)) { + throw new \Psalm\Exception\ConfigException('Unexepected error level ' . $error_level); + } + + $this->error_level = $error_level; + } + + /** + * @param string $file_name + * @return string + */ + public function getReportingLevelForFile($file_name) + { + foreach ($this->custom_levels as $custom_level) { + if ($custom_level->allows($file_name)) { + return $custom_level->getErrorLevel(); + } + } + + return $this->error_level; + } +} diff --git a/src/Psalm/Config/ProjectFileFilter.php b/src/Psalm/Config/ProjectFileFilter.php new file mode 100644 index 00000000000..954cfb5d1d4 --- /dev/null +++ b/src/Psalm/Config/ProjectFileFilter.php @@ -0,0 +1,53 @@ +ignoreFiles)) { + if (!$inclusive) { + throw new \Psalm\Exception\ConfigException('Cannot nest ignoreFiles inside itself'); + } + + /** @var \SimpleXMLElement $e->ignoreFiles */ + $filter->file_filter = static::loadFromXMLElement($e->ignoreFiles, false); + } + + return $filter; + } + + /** + * @param string $file_name + * @param boolean $case_sensitive + * @return boolean + */ + public function allows($file_name, $case_sensitive = false) + { + if ($this->inclusive) { + if ($this->file_filter) { + if (!$this->file_filter->allows($file_name, $case_sensitive)) { + return false; + } + } + } + + return parent::allows($file_name, $case_sensitive); + } +} diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index dea09246a26..57344cfeece 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -63,7 +63,7 @@ public static function add(Issue\CodeIssue $e) $error_message = $issue_type . ' - ' . $e->getShortLocation() . ' - ' . $e->getMessage(); - $reporting_level = $config->getReportingLevel($issue_type); + $reporting_level = $config->getReportingLevelForFile($issue_type, $e->getFileName()); if ($reporting_level === Config::REPORT_SUPPRESS) { return false; diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php index 75bbcffaa08..c284b0daf86 100644 --- a/tests/ArrayAccessTest.php +++ b/tests/ArrayAccessTest.php @@ -120,9 +120,7 @@ public function testInvalidArrayAccess() */ public function testMixedArrayAccess() { - $filter = new Config\FileFilter(false); - $filter->addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $context = new Context('somefile.php'); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $context = new Context('somefile.php'); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $context = new Context('somefile.php'); $stmts = self::$parser->parse('assertFalse($config->isInProjectDirs('src/ignoreme/main.php')); $this->assertFalse($config->isInProjectDirs('main.php')); } + + public function testIssueHandler() + { + $config = Config::loadFromXML('psalm.xml', ' + + + + + + + + + +'); + + $this->assertTrue($config->excludeIssueInFile('MissingReturnType', 'tests/somefile.php')); + $this->assertTrue($config->excludeIssueInFile('MissingReturnType', 'src/somefile.php')); + } + + public function testIssueHandlerWithCustomErrorLevels() + { + $config = Config::loadFromXML('psalm.xml', ' + + + + + + + + + + + + + + + + +'); + + $this->assertTrue($config->excludeIssueInFile('MissingReturnType', 'tests/somefile.php')); + $this->assertFalse($config->excludeIssueInFile('MissingReturnType', 'src/somefile.php')); + $this->assertFalse($config->excludeIssueInFile('MissingReturnType', 'src/Core/somefile.php')); + } } diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index 98cea804941..d86ccd008e6 100644 --- a/tests/FunctionCallTest.php +++ b/tests/FunctionCallTest.php @@ -45,9 +45,7 @@ function foo(int $a) : void {} */ public function testMixedArgument() { - $filter = new Config\FileFilter(false); - $filter->addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('check(); } - public function testExcludeFile() + public function testExcludeIssue() { - $filter = new Config\FileFilter(false); - $filter->addFile('somefile.php'); - Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); + Config::getInstance()->setCustomErrorLevel('UndefinedFunction', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('check(); } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage UndefinedFunction - somefile.php:2 - Function foo does not exist - */ - public function testIncludeFile() - { - $filter = new Config\FileFilter(true); - $filter->addFile('somefile.php'); - Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); - - $stmts = self::$parser->parse('check(); - - $stmts = self::$parser->parse('check(); - } - - public function testExcludeDirectory() - { - $filter = new Config\FileFilter(false); - $filter->addDirectory('src'); - Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); - - $stmts = self::$parser->parse('check(); - } - - /** - * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage UndefinedFunction - src2/somefile.php:2 - Function foo does not exist - */ - public function testIncludeDirectory() - { - $filter = new Config\FileFilter(true); - $filter->addDirectory('src2'); - Config::getInstance()->setIssueHandler('UndefinedFunction', $filter); - - (new FileChecker( - 'src1/somefile.php', - self::$parser->parse('check(); - - (new FileChecker( - 'src2/somefile.php', - self::$parser->parse('check(); - } } diff --git a/tests/MethodCallTest.php b/tests/MethodCallTest.php index b4f9e19f46a..d896d119a76 100644 --- a/tests/MethodCallTest.php +++ b/tests/MethodCallTest.php @@ -63,10 +63,8 @@ public static function bar() : void {} */ public function testMixedMethodCall() { - $filter = new Config\FileFilter(false); - $filter->addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MissingPropertyType', Config::REPORT_SUPPRESS); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MissingPropertyType', Config::REPORT_SUPPRESS); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MissingPropertyType', Config::REPORT_SUPPRESS); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('addFile('somefile.php'); - Config::getInstance()->setIssueHandler('MissingPropertyType', $filter); - Config::getInstance()->setIssueHandler('MixedAssignment', $filter); + Config::getInstance()->setCustomErrorLevel('MissingPropertyType', Config::REPORT_SUPPRESS); + Config::getInstance()->setCustomErrorLevel('MixedAssignment', Config::REPORT_SUPPRESS); $stmts = self::$parser->parse('create(ParserFactory::PREFER_PHP7); $config = new TestConfig(); - $config->throw_exception = true; - - self::$file_filter = null; } public function setUp() @@ -136,8 +133,6 @@ public function testPossiblyUndefinedArrayInWhileAndForeach() echo $array; '); - Config::getInstance()->setIssueHandler('PossiblyUndefinedVariable', self::$file_filter); - $file_checker = new FileChecker('somefile.php', $stmts); $file_checker->check(); } From ca98efb6307387f6974e36c8f8e4a9a3c6d1e964 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 20:23:04 -0500 Subject: [PATCH 5/8] Check specific reporting levels --- tests/ConfigTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index bf87e90b450..ddd699e19c3 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -87,5 +87,8 @@ public function testIssueHandlerWithCustomErrorLevels() $this->assertTrue($config->excludeIssueInFile('MissingReturnType', 'tests/somefile.php')); $this->assertFalse($config->excludeIssueInFile('MissingReturnType', 'src/somefile.php')); $this->assertFalse($config->excludeIssueInFile('MissingReturnType', 'src/Core/somefile.php')); + + $this->assertSame('info', $config->getReportingLevelForFile('MissingReturnType', 'src/somefile.php')); + $this->assertSame('error', $config->getReportingLevelForFile('MissingReturnType', 'src/Core/somefile.php')); } } From e662c420b7b1a5275ec21e2e96f27fb76a40b003 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 22:11:10 -0500 Subject: [PATCH 6/8] Add schema to verify new config format against --- config.xsd | 159 +++++++++++++++++++++++++++++++++++++++++++ src/Psalm/Config.php | 26 +++++-- 2 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 config.xsd diff --git a/config.xsd b/config.xsd new file mode 100644 index 00000000000..febb7fd4e0c --- /dev/null +++ b/config.xsd @@ -0,0 +1,159 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 6ca456155c4..ed649ac0afb 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -182,6 +182,28 @@ public static function loadFromXML($file_path, $file_contents) $config->base_dir = dirname($file_path) . '/'; + $schema_path = dirname(dirname(__DIR__)) . '/config.xsd'; + + if (!file_exists($schema_path)) { + throw new ConfigException('Cannot locate config schema'); + } + + $dom_document = new \DOMDocument(); + $dom_document->loadXML($file_contents); + + // Enable user error handling + libxml_use_internal_errors(true); + + if (!$dom_document->schemaValidate($schema_path)) { + $errors = libxml_get_errors(); + foreach ($errors as $error) { + if ($error->level === LIBXML_ERR_FATAL || $error->level === LIBXML_ERR_ERROR) { + throw new ConfigException('Error parsing file ' . $error->file . ' on line ' . $error->line . ': ' . $error->message); + } + } + libxml_clear_errors(); + } + $config_xml = new SimpleXMLElement($file_contents); if (isset($config_xml['stopOnFirstError'])) { @@ -212,10 +234,6 @@ public static function loadFromXML($file_path, $file_contents) $config->cache_directory = (string) $config_xml['cacheDirectory']; } - if (isset($config_xml['cacheDirectory'])) { - $config->cache_directory = (string) $config_xml['cacheDirectory']; - } - if (isset($config_xml['usePropertyDefaultForType'])) { $attribute_text = (string) $config_xml['usePropertyDefaultForType']; $config->use_property_default_for_type = $attribute_text === 'true' || $attribute_text === '1'; From 6236e34996a75099aa820b31b02e1877021dc91f Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 22:16:46 -0500 Subject: [PATCH 7/8] Make schema a little more lenient --- config.xsd | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/config.xsd b/config.xsd index febb7fd4e0c..a02487e7944 100644 --- a/config.xsd +++ b/config.xsd @@ -25,18 +25,18 @@ - + - + - + - + @@ -144,10 +144,10 @@ - + - + From 90fee45a1896cf67e4273e386e435c72f439299d Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 29 Dec 2016 22:31:52 -0500 Subject: [PATCH 8/8] Add test to guarantee that config schema stays up-to-date --- tests/ConfigTest.php | 57 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index ddd699e19c3..83d077a2d79 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -15,6 +15,21 @@ public function setUp() FileChecker::clearCache(); } + public static function getAllIssues() + { + return array_filter( + array_map( + function ($file_name) { + return substr($file_name, 0, -4); + }, + scandir(dirname(__DIR__) . '/src/Psalm/Issue') + ), + function ($issue_name) { + return !empty($issue_name) && $issue_name !== 'CodeError' && $issue_name !== 'CodeIssue'; + } + ); + } + public function testBarebonesConfig() { $config = Config::loadFromXML('psalm.xml', ' @@ -91,4 +106,46 @@ public function testIssueHandlerWithCustomErrorLevels() $this->assertSame('info', $config->getReportingLevelForFile('MissingReturnType', 'src/somefile.php')); $this->assertSame('error', $config->getReportingLevelForFile('MissingReturnType', 'src/Core/somefile.php')); } + + public function testAllPossibleIssues() + { + $all_possible_handlers = implode( + ' ', + array_map( + function ($issue_name) { + return '<' . $issue_name . ' errorLevel="suppress" />' . PHP_EOL; + }, + self::getAllIssues() + ) + ); + + $config = Config::loadFromXML('psalm.xml', ' + + + + + + + ' . $all_possible_handlers . ' + +'); + } + + /** + * @expectedException \Psalm\Exception\ConfigException + * @expectedExceptionMessage This element is not expected + */ + public function testImpossibleIssue() + { + $config = Config::loadFromXML('psalm.xml', ' + + + + + + + + +'); + } }