Skip to content

Commit

Permalink
provide more context for lint context
Browse files Browse the repository at this point in the history
line number and xpath
  • Loading branch information
bernt-matthias committed Nov 24, 2021
1 parent 38a69b1 commit f638f00
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 134 deletions.
80 changes: 58 additions & 22 deletions lib/galaxy/tool_util/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,19 @@ def lint_xml_with(lint_context, tool_xml, extra_modules=None):
return lint_tool_source_with(lint_context, tool_source, extra_modules=extra_modules)


class LintMessage:
def __init__(self, level, message, line, xpath=None):
self.level = level
self.message = message
self.line = line
self.xpath = xpath

def __str__(self):
rval = f".. {self.level.upper()}: {self.message}"
if self.line is not None:
rval += f" (line {self.line})"
return rval

# TODO: Nothing inherently tool-y about LintContext and in fact
# it is reused for repositories in planemo. Therefore, it should probably
# be moved to galaxy.util.lint.
Expand All @@ -80,10 +93,9 @@ def lint(self, name, lint_func, lint_target):
if name in self.skip_types:
return
self.printed_linter_info = False
self.valid_messages = []
self.info_messages = []
self.warn_messages = []
self.error_messages = []
self.message_list = []

# call linter
lint_func(lint_target, self)
# TODO: colorful emoji if in click CLI.
if self.error_messages:
Expand All @@ -99,41 +111,65 @@ def print_linter_info():
self.printed_linter_info = True
print(f"Applying linter {name}... {status}")

for message in self.error_messages:
for message in self.message_list:
if message.level != "error":
continue
self.found_errors = True
print_linter_info()
print(f".. ERROR: {message}")
print(f"{message}")

if self.level != LEVEL_ERROR:
for message in self.warn_messages:
for message in self.message_list:
if message.level != "warning":
continue
self.found_warns = True
print_linter_info()
print(f".. WARNING: {message}")
print(f"{message}")

if self.level == LEVEL_ALL:
for message in self.info_messages:
for message in self.message_list:
if message.level != "info":
continue
print_linter_info()
print(f".. INFO: {message}")
for message in self.valid_messages:
print(f"{message}")
for message in self.message_list:
if message.level != "check":
continue
print_linter_info()
print(f".. CHECK: {message}")
print(f"{message}")

@property
def valid_messages(self):
return [x.message for x in self.message_list if x.level == "check"]

@property
def info_messages(self):
return [x.message for x in self.message_list if x.level == "info"]

@property
def warn_messages(self):
return [x.message for x in self.message_list if x.level == "warning"]

def __handle_message(self, message_list, message, *args):
@property
def error_messages(self):
return [x.message for x in self.message_list if x.level == "error"]

def __handle_message(self, level, message, line, xpath, *args):
if args:
message = message % args
message_list.append(message)
self.message_list.append(LintMessage(level=level, message=message, line=line, xpath=xpath))

def valid(self, message, *args):
self.__handle_message(self.valid_messages, message, *args)
def valid(self, message, line=None, xpath=None, *args):
self.__handle_message("check", message, line, xpath, *args)

def info(self, message, *args):
self.__handle_message(self.info_messages, message, *args)
def info(self, message, line=None, xpath=None, *args):
self.__handle_message("info", message, line, xpath, *args)

def error(self, message, *args):
self.__handle_message(self.error_messages, message, *args)
def error(self, message, line=None, xpath=None, *args):
self.__handle_message("error", message, line, xpath, *args)

def warn(self, message, *args):
self.__handle_message(self.warn_messages, message, *args)
def warn(self, message, line=None, xpath=None, *args):
self.__handle_message("warning", message, line, xpath, *args)

def failed(self, fail_level):
found_warns = self.found_warns
Expand Down
12 changes: 6 additions & 6 deletions lib/galaxy/tool_util/linters/citations.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,26 @@ def lint_citations(tool_xml, lint_ctx):
root = tool_xml.getroot()
citations = root.findall("citations")
if len(citations) > 1:
lint_ctx.error("More than one citation section found, behavior undefined.")
lint_ctx.error("More than one citation section found, behavior undefined.", line=citations[1].sourceline)
return

if len(citations) == 0:
lint_ctx.warn("No citations found, consider adding citations to your tool.")
lint_ctx.warn("No citations found, consider adding citations to your tool.", line=root.sourceline)
return

valid_citations = 0
for citation in citations[0]:
if citation.tag != "citation":
lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.")
lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", line=citation.sourceline)
continue
citation_type = citation.attrib.get("type")
if citation_type not in ('bibtex', 'doi'):
lint_ctx.warn("Unknown citation type discovered [%s], will be ignored.", citation_type)
lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", line=citation.sourceline)
continue
if citation.text is None or not citation.text.strip():
lint_ctx.error(f'Empty {citation_type} citation.')
lint_ctx.error(f'Empty {citation_type} citation.', line=citation.sourceline)
continue
valid_citations += 1

if valid_citations > 0:
lint_ctx.valid("Found %d likely valid citations.", valid_citations)
lint_ctx.valid(f"Found {valid_citations} likely valid citations.", line=root.sourceline)
12 changes: 6 additions & 6 deletions lib/galaxy/tool_util/linters/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ def lint_command(tool_xml, lint_ctx):
root = tool_xml.getroot()
commands = root.findall("command")
if len(commands) > 1:
lint_ctx.error("More than one command tag found, behavior undefined.")
lint_ctx.error("More than one command tag found, behavior undefined.", line=commands[1].sourceline)
return

if len(commands) == 0:
lint_ctx.error("No command tag found, must specify a command template to execute.")
lint_ctx.error("No command tag found, must specify a command template to execute.", line=root.sourceline)
return

command = get_command(tool_xml)
if "TODO" in command:
lint_ctx.warn("Command template contains TODO text.")
lint_ctx.warn("Command template contains TODO text.", line=command.sourceline)

command_attrib = command.attrib
interpreter_type = None
Expand All @@ -29,14 +29,14 @@ def lint_command(tool_xml, lint_ctx):
elif key == "detect_errors":
detect_errors = value
if detect_errors not in ["default", "exit_code", "aggressive"]:
lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]")
lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", line=command.sourceline)

interpreter_info = ""
if interpreter_type:
interpreter_info = f" with interpreter of type [{interpreter_type}]"
if interpreter_type:
lint_ctx.info("Command uses deprecated 'interpreter' attribute.")
lint_ctx.info(f"Tool contains a command{interpreter_info}.")
lint_ctx.info("Command uses deprecated 'interpreter' attribute.", line=command.sourceline)
lint_ctx.info(f"Tool contains a command{interpreter_info}.", line=command.sourceline)


def get_command(tool_xml):
Expand Down
37 changes: 21 additions & 16 deletions lib/galaxy/tool_util/linters/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,39 +27,47 @@

def lint_general(tool_source, lint_ctx):
"""Check tool version, name, and id."""
# determine line to report for general problems with outputs
tool_xml = getattr(tool_source, "xml_tree", None)
try:
tool_line = tool_xml.find("./tool").sourceline
except:
tool_line = 0
version = tool_source.parse_version() or ''
parsed_version = packaging.version.parse(version)
if not version:
lint_ctx.error(ERROR_VERSION_MSG)
lint_ctx.error(ERROR_VERSION_MSG, line=tool_line)
elif isinstance(parsed_version, packaging.version.LegacyVersion):
lint_ctx.warn(WARN_VERSION_MSG % version)
lint_ctx.warn(WARN_VERSION_MSG % version, line=tool_line)
elif version != version.strip():
lint_ctx.warn(WARN_WHITESPACE_MSG % ('Tool version', version))
lint_ctx.warn(WARN_WHITESPACE_MSG % ('Tool version', version), line=tool_line)
else:
lint_ctx.valid(VALID_VERSION_MSG % version)
lint_ctx.valid(VALID_VERSION_MSG % version, line=tool_line)

name = tool_source.parse_name()
if not name:
lint_ctx.error(ERROR_NAME_MSG)
lint_ctx.error(ERROR_NAME_MSG, line=tool_line)
elif name != name.strip():
lint_ctx.warn(WARN_WHITESPACE_MSG % ('Tool name', name))
lint_ctx.warn(WARN_WHITESPACE_MSG % ('Tool name', name), line=tool_line)
else:
lint_ctx.valid(VALID_NAME_MSG % name)
lint_ctx.valid(VALID_NAME_MSG % name, line=tool_line)

tool_id = tool_source.parse_id()
if not tool_id:
lint_ctx.error(ERROR_ID_MSG)
lint_ctx.error(ERROR_ID_MSG, line=tool_line)
else:
lint_ctx.valid(VALID_ID_MSG % tool_id)
lint_ctx.valid(VALID_ID_MSG % tool_id, line=tool_line)
if re.search(r"\s", tool_id):
lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, line=tool_line)

profile = tool_source.parse_profile()
profile_valid = PROFILE_PATTERN.match(profile) is not None
if not profile_valid:
lint_ctx.error(PROFILE_INVALID_MSG)
lint_ctx.error(PROFILE_INVALID_MSG, line=tool_line)
elif profile == "16.01":
lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG)
lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, line=tool_line)
else:
lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile)
lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, line=tool_line)

requirements, containers = tool_source.parse_requirements_and_containers()
for r in requirements:
Expand All @@ -71,7 +79,4 @@ def lint_general(tool_source, lint_ctx):
# Warn requirement attributes with leading/trailing whitespace:
elif r.version != r.version.strip():
lint_ctx.warn(
WARN_WHITESPACE_MSG % ('Requirement version', r.version))

if re.search(r"\s", tool_id):
lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id)
WARN_WHITESPACE_MSG % ('Requirement version', r.version))
15 changes: 8 additions & 7 deletions lib/galaxy/tool_util/linters/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,32 @@

def lint_help(tool_xml, lint_ctx):
"""Ensure tool contains exactly one valid RST help block."""
# determine line to report for general problems with outputs
root = tool_xml.getroot()
helps = root.findall("help")
if len(helps) > 1:
lint_ctx.error("More than one help section found, behavior undefined.")
lint_ctx.error("More than one help section found, behavior undefined.", line=helps[1].sourceline)
return

if len(helps) == 0:
lint_ctx.warn("No help section found, consider adding a help section to your tool.")
lint_ctx.warn("No help section found, consider adding a help section to your tool.", line=root.sourceline)
return

help = helps[0].text or ''
if not help.strip():
lint_ctx.warn("Help section appears to be empty.")
lint_ctx.warn("Help section appears to be empty.", line=helps[0].sourceline)
return

lint_ctx.valid("Tool contains help section.")
lint_ctx.valid("Tool contains help section.", line=root.sourceline)
invalid_rst = rst_invalid(help)

if "TODO" in help:
lint_ctx.warn("Help contains TODO text.")
lint_ctx.warn("Help contains TODO text.", line=helps[0].sourceline)

if invalid_rst:
lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].")
lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", line=helps[0].sourceline)
else:
lint_ctx.valid("Help contains valid reStructuredText.")
lint_ctx.valid("Help contains valid reStructuredText.", line=helps[0].sourceline)


def rst_invalid(text):
Expand Down
Loading

0 comments on commit f638f00

Please sign in to comment.