half-baked remaster of error code injestion

Author: wyattscarpenter

mypy/build.py

@@ -2471,7 +2471,7 @@ def parse_inline_configuration(self, source: str) -> None:
         flags = get_mypy_comments(source)
         if flags:
             changes, config_errors = parse_mypy_comments(flags, self.options)
-            self.options = self.options.apply_changes(changes)
+            self.options = self.options.copy_with_changes(changes)
             self.manager.errors.set_file(self.xpath, self.id, self.options)
             for lineno, error in config_errors:
                 self.manager.errors.report(lineno, 0, error)
@@ -2822,8 +2822,8 @@ def dependency_lines(self) -> list[int]:
     def generate_unused_ignore_notes(self) -> None:
         if (
             self.options.warn_unused_ignores
-            or codes.UNUSED_IGNORE in self.options.enabled_error_codes
-        ) and codes.UNUSED_IGNORE not in self.options.disabled_error_codes:
+            or codes.UNUSED_IGNORE in self.options.active_error_codes
+        ):
             # If this file was initially loaded from the cache, it may have suppressed
             # dependencies due to imports with ignores on them. We need to generate
             # those errors to avoid spuriously flagging them as unused ignores.

mypy/build_worker/worker.py

@@ -75,7 +75,7 @@ def main(argv: list[str]) -> None:
     options_obj = Options()
     disable_error_code = options_dict.pop("disable_error_code", [])
     enable_error_code = options_dict.pop("enable_error_code", [])
-    options = options_obj.apply_changes(options_dict)
+    options = options_obj.copy_with_changes(options_dict)
 
     status_file = args.status_file
     server = IPCServer(CONNECTION_NAME, WORKER_CONNECTION_TIMEOUT)

mypy/checker.py

@@ -2401,7 +2401,7 @@ def check_method_override_for_base_with_name(
             if (
                 ok
                 and original_node
-                and codes.MUTABLE_OVERRIDE in self.options.enabled_error_codes
+                and codes.MUTABLE_OVERRIDE in self.options.active_error_codes
                 and self.is_writable_attribute(original_node)
                 and not always_allow_covariant
                 and not is_subtype(original_type, typ, ignore_pos_arg_names=True)
@@ -2421,7 +2421,7 @@ def check_method_override_for_base_with_name(
             # This method is a subtype of at least one union variant.
             if (
                 original_node
-                and codes.MUTABLE_OVERRIDE in self.options.enabled_error_codes
+                and codes.MUTABLE_OVERRIDE in self.options.active_error_codes
                 and self.is_writable_attribute(original_node)
                 and not always_allow_covariant
             ):
@@ -3659,7 +3659,7 @@ def check_compatibility_super(
         )
         if (
             ok
-            and codes.MUTABLE_OVERRIDE in self.options.enabled_error_codes
+            and codes.MUTABLE_OVERRIDE in self.options.active_error_codes
             and self.is_writable_attribute(base_node)
             and not always_allow_covariant
         ):

mypy/checkexpr.py

@@ -4295,7 +4295,7 @@ def check_boolean_op(self, e: OpExpr) -> Type:
         # If left_map is None then we know mypy considers the left expression
         # to be redundant.
         if (
-            codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes
+            codes.REDUNDANT_EXPR in self.chk.options.active_error_codes
             and left_map is None
             # don't report an error if it's intentional
             and not e.right_always
@@ -4783,7 +4783,7 @@ def visit_reveal_expr(self, expr: RevealExpr) -> Type:
             return NoneType()
 
     def check_reveal_imported(self, expr: RevealExpr) -> None:
-        if codes.UNIMPORTED_REVEAL not in self.chk.options.enabled_error_codes:
+        if codes.UNIMPORTED_REVEAL not in self.chk.options.active_error_codes:
             return
 
         name = ""
@@ -5911,7 +5911,7 @@ def check_for_comp(self, e: GeneratorExpr | DictionaryComprehension) -> None:
                 if true_map:
                     self.chk.push_type_map(true_map)
 
-                if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes:
+                if codes.REDUNDANT_EXPR in self.chk.options.active_error_codes:
                     if true_map is None:
                         self.msg.redundant_condition_in_comprehension(False, condition)
                     elif false_map is None:
@@ -5924,7 +5924,7 @@ def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = F
         # Gain type information from isinstance if it is there
         # but only for the current expression
         if_map, else_map = self.chk.find_isinstance_check(e.cond)
-        if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes:
+        if codes.REDUNDANT_EXPR in self.chk.options.active_error_codes:
             if if_map is None:
                 self.msg.redundant_condition_in_if(False, e.cond)
             elif else_map is None:

mypy/config_parser.py

@@ -336,6 +336,8 @@ def parse_config_file(
         updates, report_dirs = parse_section(
             prefix, options, set_strict_flags, section, config_types, stderr
         )
+        #TODO(Wyatt): should this be `options = options.copy_with_changes(updates)`,
+        # or did trying that break everything?
         for k, v in updates.items():
             setattr(options, k, v)
         options.report_dirs.update(report_dirs)
@@ -495,9 +497,11 @@ def parse_section(
     results: dict[str, object] = {}
     report_dirs: dict[str, str] = {}
 
-    # Because these fields exist on Options, without proactive checking, we would accept them
-    # and crash later
     invalid_options = {
+        # Because this field exists on Options,
+        # without this proactive checking we would accept it and crash later:
+        "active_error_codes": "enable_error_code",
+        # These used to also be fields on Options, but now are just helpful tips:
         "enabled_error_codes": "enable_error_code",
         "disabled_error_codes": "disable_error_code",
     }
@@ -579,13 +583,6 @@ def parse_section(
                 set_strict_flags()
             continue
         results[options_key] = v
-
-    # These two flags act as per-module overrides, so store the empty defaults.
-    if "disable_error_code" not in results:
-        results["disable_error_code"] = []
-    if "enable_error_code" not in results:
-        results["enable_error_code"] = []
-
     return results, report_dirs
 
 
@@ -694,24 +691,9 @@ def set_strict_flags() -> None:
                     '(see "mypy -h" for the list of flags enabled in strict mode)',
                 )
             )
-        # Because this is currently special-cased
-        # (the new_sections for an inline config *always* includes 'disable_error_code' and
-        # 'enable_error_code' fields, usually empty, which overwrite the old ones),
-        # we have to manipulate them specially.
-        # This could use a refactor, but so could the whole subsystem.
-        if (
-            "enable_error_code" in new_sections
-            and isinstance(neec := new_sections["enable_error_code"], list)
-            and isinstance(eec := sections.get("enable_error_code", []), list)
-        ):
-            new_sections["enable_error_code"] = sorted(set(neec + eec))
-        if (
-            "disable_error_code" in new_sections
-            and isinstance(ndec := new_sections["disable_error_code"], list)
-            and isinstance(dec := sections.get("disable_error_code", []), list)
-        ):
-            new_sections["disable_error_code"] = sorted(set(ndec + dec))
+
         sections.update(new_sections)
+
     return sections, errors
 
 

mypy/dmypy/client.py

@@ -622,7 +622,7 @@ def do_daemon(args: argparse.Namespace) -> None:
 
         options_dict = pickle.loads(base64.b64decode(args.options_data))
         options_obj = Options()
-        options = options_obj.apply_changes(options_dict)
+        options = options_obj.copy_with_changes(options_dict)
     else:
         options = process_start_options(args.flags, allow_sources=False)
 

mypy/errorcodes.py

@@ -11,9 +11,14 @@
 from mypy_extensions import mypyc_attr
 
 error_codes: dict[str, ErrorCode] = {}
+error_codes_on_by_default: Final[set[ErrorCode]] = set()
 sub_code_map: dict[str, set[str]] = defaultdict(set)
 
 
+def print_code_set(error_codes: set[ErrorCode]) -> None:
+    print(sorted([e.code for e in error_codes]))
+
+
 @mypyc_attr(allow_interpreted_subclasses=True)
 class ErrorCode:
     def __init__(
@@ -33,6 +38,8 @@ def __init__(
             assert sub_code_of.sub_code_of is None, "Nested subcategories are not supported"
             sub_code_map[sub_code_of.code].add(code)
         error_codes[code] = self
+        if default_enabled:
+            error_codes_on_by_default.add(self)
 
     def __str__(self) -> str:
         return f"<ErrorCode {self.code}>"
@@ -336,3 +343,4 @@ def __hash__(self) -> int:
 
 # This copy will not include any error codes defined later in the plugins.
 mypy_error_codes = error_codes.copy()
+mypy_error_codes_on_by_default = error_codes_on_by_default.copy()

mypy/errors.py

@@ -829,21 +829,7 @@ def is_ignored_error(self, line: int, info: ErrorInfo, ignores: dict[int, list[s
         return False
 
     def is_error_code_enabled(self, error_code: ErrorCode) -> bool:
-        if self.options:
-            current_mod_disabled = self.options.disabled_error_codes
-            current_mod_enabled = self.options.enabled_error_codes
-        else:
-            current_mod_disabled = set()
-            current_mod_enabled = set()
-
-        if error_code in current_mod_disabled:
-            return False
-        elif error_code in current_mod_enabled:
-            return True
-        elif error_code.sub_code_of is not None and error_code.sub_code_of in current_mod_disabled:
-            return False
-        else:
-            return error_code.default_enabled
+        return error_code in self.options.active_error_codes
 
     def clear_errors_in_targets(self, path: str, targets: set[str]) -> None:
         """Remove errors in specific fine-grained targets within a file."""

mypy/options.py

@@ -9,7 +9,7 @@
 from typing import Any, Final
 
 from mypy import defaults
-from mypy.errorcodes import ErrorCode, error_codes
+from mypy.errorcodes import ErrorCode, error_codes, error_codes_on_by_default
 from mypy.util import get_class_descriptors, replace_object_state
 
 
@@ -29,7 +29,6 @@ class BuildType:
     "check_untyped_defs",
     "debug_cache",
     "disable_error_code",
-    "disabled_error_codes",
     "disallow_any_decorated",
     "disallow_any_explicit",
     "disallow_any_expr",
@@ -41,7 +40,6 @@ class BuildType:
     "disallow_untyped_decorators",
     "disallow_untyped_defs",
     "enable_error_code",
-    "enabled_error_codes",
     "extra_checks",
     "follow_imports_for_stubs",
     "follow_imports",
@@ -254,13 +252,15 @@ def __init__(self) -> None:
         # Variable names considered False
         self.always_false: list[str] = []
 
-        # Error codes to disable
+        # The set of error codes (as ErrorCode objects) we've computed as
+        # being actually enabled or disabled after all is said and done.
+        self.active_error_codes: set[ErrorCode] = error_codes_on_by_default.copy()
+
+        # List of error codes we are being instructed to disable
         self.disable_error_code: list[str] = []
-        self.disabled_error_codes: set[ErrorCode] = set()
 
-        # Error codes to enable
+        # List of error codes we are being instructed to enable
         self.enable_error_code: list[str] = []
-        self.enabled_error_codes: set[ErrorCode] = set()
 
         # Use script name instead of __main__
         self.scripts_are_modules = False
@@ -437,7 +437,11 @@ def __repr__(self) -> str:
         return f"Options({pprint.pformat(self.snapshot())})"
 
     def process_error_codes(self, *, error_callback: Callable[[str], Any]) -> None:
-        """Process `--enable-error-code` and `--disable-error-code` flags."""
+        # This function seems to be standalone so that it can be called after plugins, in various places.
+        """Process `enable-error-code` and `disable-error-code` flags.
+        This clears those fields, which were temporary, and only used for option injestion.
+        The persistant field to consult about error code enablement is active_error_code.
+        """
         disabled_code_names = set(self.disable_error_code)
         enabled_code_names = set(self.enable_error_code)
 
@@ -448,12 +452,11 @@ def process_error_codes(self, *, error_callback: Callable[[str], Any]) -> None:
         ) - valid_error_code_names
         if invalid_code_names_here:
             error_callback(f"Invalid error code(s): {', '.join(sorted(invalid_code_names_here))}")
-
-        self.disabled_error_codes |= {error_codes[code] for code in disabled_code_names}
-        self.enabled_error_codes |= {error_codes[code] for code in enabled_code_names}
-
-        # Enabling an error code always overrides disabling
-        self.disabled_error_codes -= self.enabled_error_codes
+#TODO(Wyatt): I don't think the new way accounts for subcodes
+        self.active_error_codes -= {error_codes[code] for code in disabled_code_names}
+        self.active_error_codes |= {error_codes[code] for code in enabled_code_names}
+        self.disable_error_code.clear()
+        self.enable_error_code.clear()
 
     def process_incomplete_features(
         self, *, error_callback: Callable[[str], Any], warning_callback: Callable[[str], Any]
@@ -475,41 +478,32 @@ def process_strict_bytes(self) -> None:
             # forwards compatibility
             self.strict_bytes = True
 
-    def apply_changes(self, changes: dict[str, object]) -> Options:
-        # Note: effects of this method *must* be idempotent.
+    def copy_with_changes(self, changes: dict[str, object]) -> Options:
+        """Return a new Options object that has the changes applied over the old one.
+        Note: effects of this method *must* be idempotent."""
         new_options = Options()
         # Under mypyc, we don't have a __dict__, so we need to do worse things.
         replace_object_state(new_options, self, copy_dict=True)
         for key, value in changes.items():
             setattr(new_options, key, value)
         if changes.get("ignore_missing_imports"):
             # This is the only option for which a per-module and a global
-            # option sometimes beheave differently.
+            # option sometimes behave differently.
             new_options.ignore_missing_imports_per_module = True
-
-        # These two act as overrides, so apply them when cloning.
-        # Similar to global codes enabling overrides disabling, so we start from latter.
-        new_options.disabled_error_codes = self.disabled_error_codes.copy()
-        new_options.enabled_error_codes = self.enabled_error_codes.copy()
-        for code_str in new_options.disable_error_code:
-            code = error_codes[code_str]
-            new_options.disabled_error_codes.add(code)
-            new_options.enabled_error_codes.discard(code)
-        for code_str in new_options.enable_error_code:
-            code = error_codes[code_str]
-            new_options.enabled_error_codes.add(code)
-            new_options.disabled_error_codes.discard(code)
+        new_options.process_error_codes(
+            error_callback=lambda x: print(x, sys.stderr and exit(x)) if x else None
+        )
         return new_options
 
     def compare_stable(self, other_snapshot: dict[str, object]) -> bool:
-        """Compare options in a way that is stable for snapshot() -> apply_changes() roundtrip.
+        """Compare options in a way that is stable for snapshot() -> copy_with_changes() roundtrip.
 
-        This is needed because apply_changes() has non-trivial effects for some flags, so
-        Options().apply_changes(options.snapshot()) may result in a (slightly) different object.
+        This is needed because copy_with_changes() has non-trivial effects for some flags, so
+        Options().copy_with_changes(options.snapshot()) may result in a (slightly) different object.
         """
         return (
-            Options().apply_changes(self.snapshot()).snapshot()
-            == Options().apply_changes(other_snapshot).snapshot()
+            Options().copy_with_changes(self.snapshot()).snapshot()
+            == Options().copy_with_changes(other_snapshot).snapshot()
         )
 
     def build_per_module_cache(self) -> None:
@@ -549,7 +543,7 @@ def build_per_module_cache(self) -> None:
             # on inheriting from parent configs.
             options = self.clone_for_module(key)
             # And then update it with its per-module options.
-            self._per_module_cache[key] = options.apply_changes(self.per_module_options[key])
+            self._per_module_cache[key] = options.copy_with_changes(self.per_module_options[key])
 
         # Add the more structured sections into unused configs, since
         # they only count as used if actually used by a real module.
@@ -590,7 +584,7 @@ def clone_for_module(self, module: str) -> Options:
             for key, pattern in self._glob_options:
                 if pattern.match(module):
                     self.unused_configs.discard(key)
-                    options = options.apply_changes(self.per_module_options[key])
+                    options = options.copy_with_changes(self.per_module_options[key])
 
         # We could update the cache to directly point to modules once
         # they have been looked up, but in testing this made things
@@ -612,7 +606,7 @@ def select_options_affecting_cache(self) -> dict[str, object]:
         result: dict[str, object] = {}
         for opt in OPTIONS_AFFECTING_CACHE:
             val = getattr(self, opt)
-            if opt in ("disabled_error_codes", "enabled_error_codes"):
+            if opt == "active_error_codes":
                 val = sorted([code.code for code in val])
             result[opt] = val
         return result

mypy/test/testparse.py

@@ -56,7 +56,7 @@ def test_parser(testcase: DataDrivenTestCase) -> None:
     # Apply mypy: comments to options.
     comments = get_mypy_comments(source)
     changes, _ = parse_mypy_comments(comments, options)
-    options = options.apply_changes(changes)
+    options = options.copy_with_changes(changes)
 
     try:
         n = parse(