Review comments and clean up

Improve documentation english wording
Remove state mutation on ErrorCode and move back to ErrorCode set approach
Add test cases for commandline exit for invalid codes
Decrease parser error message verbosity for invalid codes

Author: cph-w

docs/source/command_line.rst

@@ -572,7 +572,7 @@ of the above sections.
 
     This flag allows enabling one or multiple error codes globally.
 
-    NOTE: Will override disabled error codes from the --disable-error-code
+    Note: This flag will override disabled error codes from the --disable-error-code
     flag
 
     .. code-block:: python

mypy/build.py

@@ -223,7 +223,9 @@ def _build(sources: List[BuildSource],
                     options.show_error_codes,
                     options.pretty,
                     lambda path: read_py_file(path, cached_read, options.python_version),
-                    options.show_absolute_path)
+                    options.show_absolute_path,
+                    options.enabled_error_codes,
+                    options.disabled_error_codes)
     plugin, snapshot = load_plugins(options, errors, stdout, extra_plugins)
 
     # Add catch-all .gitignore to cache dir if we created it

mypy/errorcodes.py

@@ -22,7 +22,6 @@ def __init__(self, code: str,
         self.description = description
         self.category = category
         self.default_enabled = default_enabled
-        self.enabled = default_enabled
         error_codes[code] = self
 
     def __str__(self) -> str:

mypy/errors.py

@@ -164,14 +164,18 @@ def __init__(self,
                  show_error_codes: bool = False,
                  pretty: bool = False,
                  read_source: Optional[Callable[[str], Optional[List[str]]]] = None,
-                 show_absolute_path: bool = False) -> None:
+                 show_absolute_path: bool = False,
+                 enabled_error_codes: Optional[Set[ErrorCode]] = None,
+                 disabled_error_codes: Optional[Set[ErrorCode]] = None) -> None:
         self.show_error_context = show_error_context
         self.show_column_numbers = show_column_numbers
         self.show_error_codes = show_error_codes
         self.show_absolute_path = show_absolute_path
         self.pretty = pretty
         # We use fscache to read source code when showing snippets.
         self.read_source = read_source
+        self.enabled_error_codes = enabled_error_codes or set()
+        self.disabled_error_codes = disabled_error_codes or set()
         self.initialize()
 
     def initialize(self) -> None:
@@ -195,7 +199,9 @@ def copy(self) -> 'Errors':
                      self.show_error_codes,
                      self.pretty,
                      self.read_source,
-                     self.show_absolute_path)
+                     self.show_absolute_path,
+                     self.enabled_error_codes,
+                     self.disabled_error_codes)
         new.file = self.file
         new.import_ctx = self.import_ctx[:]
         new.function_or_member = self.function_or_member[:]
@@ -351,17 +357,25 @@ def add_error_info(self, info: ErrorInfo) -> None:
         self._add_error_info(file, info)
 
     def is_ignored_error(self, line: int, info: ErrorInfo, ignores: Dict[int, List[str]]) -> bool:
-        if info.code and info.code.enabled is False:
+        if info.code and self.is_error_code_enabled(info.code) is False:
             return True
         elif line not in ignores:
             return False
         elif not ignores[line]:
             # Empty list means that we ignore all errors
             return True
-        elif info.code:
+        elif info.code and self.is_error_code_enabled(info.code) is True:
             return info.code.code in ignores[line]
         return False
 
+    def is_error_code_enabled(self, error_code: ErrorCode) -> bool:
+        if error_code in self.disabled_error_codes:
+            return False
+        elif error_code in self.enabled_error_codes:
+            return True
+        else:
+            return error_code.default_enabled
+
     def clear_errors_in_targets(self, path: str, targets: Set[str]) -> None:
         """Remove errors in specific fine-grained targets within a file."""
         if path in self.error_info_map:

mypy/main.py

@@ -7,7 +7,7 @@
 import sys
 import time
 
-from typing import Any, Dict, IO, Iterable, List, Optional, Sequence, Tuple, TextIO, Union
+from typing import Any, Dict, IO, List, Optional, Sequence, Tuple, TextIO, Union
 from typing_extensions import Final, NoReturn
 
 from mypy import build
@@ -869,24 +869,22 @@ def set_strict_flags() -> None:
         parser.error("You can't make a variable always true and always false (%s)" %
                      ', '.join(sorted(overlap)))
 
-    # Process disabled (--disable-error-code) codes
+    # Process `--enable-error-code` and `--disable-error-code` flags
     disabled_codes = set(options.disable_error_code)
-    if disabled_codes:
-        invalid_error_codes = process_error_codes(disabled_codes, enable=False)
+    enabled_codes = set(options.enable_error_code)
 
-        if invalid_error_codes:
-            parser.error("Disabled error codes provided invalid values: (%s)" %
-                         ', '.join(sorted(invalid_error_codes)))
+    valid_error_codes = set(error_codes.keys())
 
-    # Process enabled (--enable-error-code) codes
-    # If a flag is both enabled and disabled the result is enabled
-    enabled_codes = set(options.enable_error_code)
-    if enabled_codes:
-        invalid_error_codes = process_error_codes(enabled_codes, enable=True)
+    invalid_codes = (enabled_codes | disabled_codes) - valid_error_codes
+    if invalid_codes:
+        parser.error("Invalid error code(s): %s" %
+                     ', '.join(sorted(invalid_codes)))
 
-        if invalid_error_codes:
-            parser.error("Enabled error codes provided invalid values: (%s)" %
-                ', '.join(sorted(invalid_error_codes)))
+    options.disabled_error_codes |= {error_codes[code] for code in disabled_codes}
+    options.enabled_error_codes |= {error_codes[code] for code in enabled_codes}
+
+    # Enabling an error code always overrides disabling
+    options.disabled_error_codes -= options.enabled_error_codes
 
     # Set build flags.
     if options.strict_optional_whitelist is not None:
@@ -1012,20 +1010,6 @@ def process_cache_map(parser: argparse.ArgumentParser,
         options.cache_map[source] = (meta_file, data_file)
 
 
-def process_error_codes(codes: Iterable[str], enable: bool) -> List[str]:
-    invalid_error_codes = []  # type: List[str]
-
-    # process globally disabled error codes
-    for code in codes:
-        as_code = error_codes.get(code)
-        if not as_code:
-            invalid_error_codes.append(code)
-            continue
-        as_code.enabled = enable
-
-    return invalid_error_codes
-
-
 def maybe_write_junit_xml(td: float, serious: bool, messages: List[str], options: Options) -> None:
     if options.junit_xml:
         py_version = '{}_{}'.format(options.python_version[0], options.python_version[1])

mypy/options.py

@@ -3,12 +3,15 @@
 import pprint
 import sys
 
-from typing_extensions import Final
+from typing_extensions import Final, TYPE_CHECKING
 from typing import Dict, List, Mapping, Optional, Pattern, Set, Tuple, Callable, Any
 
 from mypy import defaults
 from mypy.util import get_class_descriptors, replace_object_state
 
+if TYPE_CHECKING:
+    from mypy.errors import ErrorCode
+
 
 class BuildType:
     STANDARD = 0  # type: Final[int]
@@ -179,9 +182,11 @@ def __init__(self) -> None:
 
         # Error codes to disable
         self.disable_error_code = []  # type: List[str]
+        self.disabled_error_codes = set()  # type: Set[ErrorCode]
 
         # Error codes to enable
         self.enable_error_code = []  # type: List[str]
+        self.enabled_error_codes = set()  # type: Set[ErrorCode]
 
         # Use script name instead of __main__
         self.scripts_are_modules = False

mypy/test/testcheck.py

@@ -18,7 +18,6 @@
     retry_on_error, update_testcase_output, parse_options,
     copy_and_fudge_mtime, assert_target_equivalence, check_test_output_files
 )
-from mypy import errorcodes
 from mypy.errors import CompileError
 from mypy.semanal_main import core_modules
 
@@ -254,15 +253,6 @@ def run_case_once(self, testcase: DataDrivenTestCase,
         if testcase.output_files:
             check_test_output_files(testcase, incremental_step, strip_prefix='tmp/')
 
-        # Post run cleanup step to reset the state of the ErrorCodes that may
-        # have been enabled or disabled by flags
-        if options.disable_error_code or options.enable_error_code:
-            altered_error_codes = set(
-                options.disable_error_code).union(set(options.enable_error_code))
-            for code in altered_error_codes:
-                error_code = errorcodes.error_codes[code]
-                error_code.enabled = error_code.default_enabled
-
     def verify_cache(self, module_data: List[Tuple[str, str, str]], a: List[str],
                      manager: build.BuildManager, graph: Graph) -> None:
         # There should be valid cache metadata for each module except

test-data/unit/cmdline.test

@@ -1092,3 +1092,53 @@ import foo.bar
 [out]
 src/foo/bar.py: error: Source file found twice under different module names: 'src.foo.bar' and 'foo.bar'
 == Return code: 2
+
+[case testEnableInvalidErrorCode]
+# cmd: mypy --enable-error-code YOLO test.py
+[file test.py]
+x = 1
+[out]
+usage: mypy [-h] [-v] [-V] [more options; see below]
+            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
+mypy: error: Invalid error code(s): YOLO
+== Return code: 2
+
+[case testDisableInvalidErrorCode]
+# cmd: mypy --disable-error-code YOLO test.py
+[file test.py]
+x = 1
+[out]
+usage: mypy [-h] [-v] [-V] [more options; see below]
+            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
+mypy: error: Invalid error code(s): YOLO
+== Return code: 2
+
+[case testEnableAndDisableInvalidErrorCode]
+# cmd: mypy --disable-error-code YOLO --enable-error-code YOLO2 test.py
+[file test.py]
+x = 1
+[out]
+usage: mypy [-h] [-v] [-V] [more options; see below]
+            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
+mypy: error: Invalid error code(s): YOLO, YOLO2
+== Return code: 2
+
+[case testEnableValidAndInvalidErrorCode]
+# cmd: mypy --enable-error-code attr-defined --enable-error-code YOLO test.py
+[file test.py]
+x = 1
+[out]
+usage: mypy [-h] [-v] [-V] [more options; see below]
+            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
+mypy: error: Invalid error code(s): YOLO
+== Return code: 2
+
+[case testDisableValidAndInvalidErrorCode]
+# cmd: mypy --disable-error-code attr-defined --disable-error-code YOLO test.py
+[file test.py]
+x = 1
+[out]
+usage: mypy [-h] [-v] [-V] [more options; see below]
+            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
+mypy: error: Invalid error code(s): YOLO
+== Return code: 2