Skip to content

Add beginnings of error code support #7267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Aug 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ def _build(sources: List[BuildSource],
reports = Reports(data_dir, options.report_dirs)

source_set = BuildSourceSet(sources)
errors = Errors(options.show_error_context, options.show_column_numbers)
errors = Errors(options.show_error_context,
options.show_column_numbers,
options.show_error_codes)
plugin, snapshot = load_plugins(options, errors, stdout)

# Construct a build manager object to hold state during the build.
Expand Down
50 changes: 50 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""Classification of possible errors mypy can detect.

These can be used for filtering specific errors.
"""

from typing import List
from typing_extensions import Final


# All created error codes are implicitly stored in this list.
all_error_codes = [] # type: List[ErrorCode]


class ErrorCode:
def __init__(self, code: str, description: str, category: str) -> None:
self.code = code
self.description = description
self.category = category
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the plans for category?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea is to use it for grouping the available error codes in the output from --show-error-codes, once that's implemented. This way error codes defined by plugins, for example, can be shown in a separate section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually meant a separate command such as --list-error-codes that shows all the available error codes.


def __str__(self) -> str:
return '<ErrorCode {}>'.format(self.code)


ATTR_DEFINED = ErrorCode(
'attr-defined', "Check that attribute exists", 'General') # type: Final
NAME_DEFINED = ErrorCode(
'name-defined', "Check that name is defined", 'General') # type: Final
CALL_ARG = ErrorCode(
'call-arg', "Check number, names and kinds of arguments in calls", 'General') # type: Final
ARG_TYPE = ErrorCode(
'arg-type', "Check argument types in calls", 'General') # type: Final
VALID_TYPE = ErrorCode(
'valid-type', "Check that type (annotation) is valid", 'General') # type: Final
MISSING_ANN = ErrorCode(
'var-annotated', "Require variable annotation if type can't be inferred",
'General') # type: Final
OVERRIDE = ErrorCode(
'override', "Check that method override is compatible with base class",
'General') # type: Final
RETURN_VALUE = ErrorCode(
'return-value', "Check that return value is compatible with signature",
'General') # type: Final
ASSIGNMENT = ErrorCode(
'assignment', "Check that assigned value is compatible with target", 'General') # type: Final

SYNTAX = ErrorCode(
'syntax', "Report syntax errors", 'General') # type: Final

MISC = ErrorCode(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One design question is whether we care about a backwards compatibility story as uncategorized errors get moved from misc to specific categories (assuming we don't categorize everything immediately), breaking type ignores that ignored misc.
Some options here:

  1. Don't care, we break backwards compat all the time
  2. Make misc a catch-all group that encompasses either everything else or everything else that used to be included under misc
  3. Rename misc to something that makes it more clear it is ephermeral, like uncategorized
  4. Disallow # type: ignore[misc]

I'm partial to 3 and maybe also 4.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal is to categorize all common errors so that the misc code should not be needed that often, making the backwards compatibility issue less of a problem. For example, if error codes could cover ~98% of errors, all the remaining errors in the misc category would each be relatively rare, so changing some of them to non-misc codes should not have a big impact.

'misc', "Miscenallenous other checks", 'General') # type: Final
91 changes: 64 additions & 27 deletions mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from mypy.scope import Scope
from mypy.options import Options
from mypy.version import __version__ as mypy_version
from mypy.errorcodes import ErrorCode
from mypy import errorcodes as codes

T = TypeVar('T')
allowed_duplicates = ['@overload', 'Got:', 'Expected:'] # type: Final
Expand Down Expand Up @@ -45,6 +47,9 @@ class ErrorInfo:
# The error message.
message = ''

# The error code.
code = None # type: Optional[ErrorCode]

# If True, we should halt build after the file that generated this error.
blocker = False

Expand All @@ -68,6 +73,7 @@ def __init__(self,
column: int,
severity: str,
message: str,
code: Optional[ErrorCode],
blocker: bool,
only_once: bool,
origin: Optional[Tuple[str, int, int]] = None,
Expand All @@ -81,12 +87,23 @@ def __init__(self,
self.column = column
self.severity = severity
self.message = message
self.code = code
self.blocker = blocker
self.only_once = only_once
self.origin = origin or (file, line, line)
self.target = target


# Type used internally to represent errors:
# (path, line, column, severity, message, code)
ErrorTuple = Tuple[Optional[str],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this a namedtuple?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do it, but since we never access the items by index, the benefit would be minor, I think, so I'd rather keep it as it is.

int,
int,
str,
str,
Optional[ErrorCode]]


class Errors:
"""Container for compile errors.

Expand All @@ -111,8 +128,9 @@ class Errors:
# Path to current file.
file = '' # type: str

# Ignore errors on these lines of each file.
ignored_lines = None # type: Dict[str, Set[int]]
# Ignore some errors on these lines of each file
# (path -> line -> error-codes)
ignored_lines = None # type: Dict[str, Dict[int, List[str]]]

# Lines on which an error was actually ignored.
used_ignored_lines = None # type: Dict[str, Set[int]]
Expand All @@ -135,10 +153,13 @@ class Errors:
target_module = None # type: Optional[str]
scope = None # type: Optional[Scope]

def __init__(self, show_error_context: bool = False,
show_column_numbers: bool = False) -> None:
def __init__(self,
show_error_context: bool = False,
show_column_numbers: bool = False,
show_error_codes: bool = False) -> None:
self.show_error_context = show_error_context
self.show_column_numbers = show_column_numbers
self.show_error_codes = show_error_codes
self.initialize()

def initialize(self) -> None:
Expand Down Expand Up @@ -197,7 +218,7 @@ def set_file(self, file: str,
self.scope = scope

def set_file_ignored_lines(self, file: str,
ignored_lines: Set[int],
ignored_lines: Dict[int, List[str]],
ignore_all: bool = False) -> None:
self.ignored_lines[file] = ignored_lines
if ignore_all:
Expand Down Expand Up @@ -226,6 +247,8 @@ def report(self,
line: int,
column: Optional[int],
message: str,
code: Optional[ErrorCode] = None,
*,
blocker: bool = False,
severity: str = 'error',
file: Optional[str] = None,
Expand All @@ -237,7 +260,9 @@ def report(self,

Args:
line: line number of error
column: column number of error
message: message to report
code: error code (defaults to 'misc' for 'error' severity)
blocker: if True, don't continue analysis after this error
severity: 'error' or 'note'
file: if non-None, override current file as context
Expand Down Expand Up @@ -267,8 +292,11 @@ def report(self,
if end_line is None:
end_line = origin_line

if severity == 'error' and code is None:
code = codes.MISC

info = ErrorInfo(self.import_context(), file, self.current_module(), type,
function, line, column, severity, message,
function, line, column, severity, message, code,
blocker, only_once,
origin=(self.file, origin_line, end_line),
target=self.current_target())
Expand All @@ -293,7 +321,7 @@ def add_error_info(self, info: ErrorInfo) -> None:
# Check each line in this context for "type: ignore" comments.
# line == end_line for most nodes, so we only loop once.
for scope_line in range(line, end_line + 1):
if scope_line in self.ignored_lines[file]:
if self.is_ignored_error(scope_line, info, self.ignored_lines[file]):
# Annotation requests us to ignore all errors on this line.
self.used_ignored_lines[file].add(scope_line)
return
Expand All @@ -305,6 +333,16 @@ def add_error_info(self, info: ErrorInfo) -> None:
self.only_once_messages.add(info.message)
self._add_error_info(file, info)

def is_ignored_error(self, line: int, info: ErrorInfo, ignores: Dict[int, List[str]]) -> bool:
if line not in ignores:
return False
elif not ignores[line]:
# Empty list means that we ignore all errors
return True
elif info.code:
return info.code.code in ignores[line]
return False

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:
Expand All @@ -319,11 +357,11 @@ def clear_errors_in_targets(self, path: str, targets: Set[str]) -> None:
def generate_unused_ignore_errors(self, file: str) -> None:
ignored_lines = self.ignored_lines[file]
if not self.is_typeshed_file(file) and file not in self.ignored_files:
for line in ignored_lines - self.used_ignored_lines[file]:
for line in set(ignored_lines) - self.used_ignored_lines[file]:
# Don't use report since add_error_info will ignore the error!
info = ErrorInfo(self.import_context(), file, self.current_module(), None,
None, line, -1, 'error', "unused 'type: ignore' comment",
False, False)
None, False, False)
self._add_error_info(file, info)

def is_typeshed_file(self, file: str) -> bool:
Expand Down Expand Up @@ -373,7 +411,7 @@ def format_messages(self, error_info: List[ErrorInfo]) -> List[str]:
a = [] # type: List[str]
errors = self.render_messages(self.sort_messages(error_info))
errors = self.remove_duplicates(errors)
for file, line, column, severity, message in errors:
for file, line, column, severity, message, code in errors:
s = ''
if file is not None:
if self.show_column_numbers and line >= 0 and column >= 0:
Expand All @@ -385,6 +423,8 @@ def format_messages(self, error_info: List[ErrorInfo]) -> List[str]:
s = '{}: {}: {}'.format(srcloc, severity, message)
else:
s = message
if self.show_error_codes and code:
s = '{} [{}]'.format(s, code.code)
a.append(s)
return a

Expand Down Expand Up @@ -420,18 +460,16 @@ def targets(self) -> Set[str]:
for info in errs
if info.target)

def render_messages(self, errors: List[ErrorInfo]) -> List[Tuple[Optional[str], int, int,
str, str]]:
def render_messages(self,
errors: List[ErrorInfo]) -> List[ErrorTuple]:
"""Translate the messages into a sequence of tuples.

Each tuple is of form (path, line, col, severity, message).
Each tuple is of form (path, line, col, severity, message, code).
The rendered sequence includes information about error contexts.
The path item may be None. If the line item is negative, the
line number is not defined for the tuple.
"""
result = [] # type: List[Tuple[Optional[str], int, int, str, str]]
# (path, line, column, severity, message)

result = [] # type: List[ErrorTuple]
prev_import_context = [] # type: List[Tuple[str, int]]
prev_function_or_member = None # type: Optional[str]
prev_type = None # type: Optional[str]
Expand All @@ -455,7 +493,7 @@ def render_messages(self, errors: List[ErrorInfo]) -> List[Tuple[Optional[str],
# Remove prefix to ignore from path (if present) to
# simplify path.
path = remove_path_prefix(path, self.ignore_prefix)
result.append((None, -1, -1, 'note', fmt.format(path, line)))
result.append((None, -1, -1, 'note', fmt.format(path, line), None))
i -= 1

file = self.simplify_path(e.file)
Expand All @@ -467,27 +505,27 @@ def render_messages(self, errors: List[ErrorInfo]) -> List[Tuple[Optional[str],
e.type != prev_type):
if e.function_or_member is None:
if e.type is None:
result.append((file, -1, -1, 'note', 'At top level:'))
result.append((file, -1, -1, 'note', 'At top level:', None))
else:
result.append((file, -1, -1, 'note', 'In class "{}":'.format(
e.type)))
e.type), None))
else:
if e.type is None:
result.append((file, -1, -1, 'note',
'In function "{}":'.format(
e.function_or_member)))
e.function_or_member), None))
else:
result.append((file, -1, -1, 'note',
'In member "{}" of class "{}":'.format(
e.function_or_member, e.type)))
e.function_or_member, e.type), None))
elif e.type != prev_type:
if e.type is None:
result.append((file, -1, -1, 'note', 'At top level:'))
result.append((file, -1, -1, 'note', 'At top level:', None))
else:
result.append((file, -1, -1, 'note',
'In class "{}":'.format(e.type)))
'In class "{}":'.format(e.type), None))

result.append((file, e.line, e.column, e.severity, e.message))
result.append((file, e.line, e.column, e.severity, e.message, e.code))

prev_import_context = e.import_ctx
prev_function_or_member = e.function_or_member
Expand Down Expand Up @@ -518,10 +556,9 @@ def sort_messages(self, errors: List[ErrorInfo]) -> List[ErrorInfo]:
result.extend(a)
return result

def remove_duplicates(self, errors: List[Tuple[Optional[str], int, int, str, str]]
) -> List[Tuple[Optional[str], int, int, str, str]]:
def remove_duplicates(self, errors: List[ErrorTuple]) -> List[ErrorTuple]:
"""Remove duplicates from a sorted error list."""
res = [] # type: List[Tuple[Optional[str], int, int, str, str]]
res = [] # type: List[ErrorTuple]
i = 0
while i < len(errors):
dup = False
Expand Down
Loading