Skip to content

Commit

Permalink
👌 Improve sphinx warnings (#975)
Browse files Browse the repository at this point in the history
This PR is intended to standardise and improve the sphinx warnings
emitted by sphinx-needs:

1. To make it clear when a warning originates from sphinx-needs
2. Where possible, to add the location of the warning origin (in the
source documentation)

i.e. all warning logs should now look like:

```
/path/to/file.rst:<line number>: WARNING: <message> [needs]
```

---

This is similar to my work in
https://myst-parser.readthedocs.io/en/latest/configuration.html#build-warnings,
it prefixes all warnings with `[needs]`, and adds the `needs` type to
the warning, meaning it can be specifically suppressed (see
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-suppress_warnings).
(warning subtypes could also be added in follow up PRs)

I have also added the `location` argument to some warnings (where the
location was obvious), although there are probably some more that could
also have a location (but this can be done in later PRs),

Note, IMO it would be ideal if the type was automatically shown by
sphinx, but this is not currently the case:
sphinx-doc/sphinx#8845

Note also, I intend to make an upstream PR to
https://github.com/sphinx-contrib/plantuml, to also improve their
warnings
  • Loading branch information
chrisjsewell authored Aug 21, 2023
1 parent 78413f6 commit 5b983a4
Show file tree
Hide file tree
Showing 24 changed files with 114 additions and 66 deletions.
2 changes: 2 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ Released: under development

* Change `NeedsBuilder` format to `needs` (`#978 <https://github.com/useblocks/sphinx-needs/pull/978>`_)

* Improvement: Suffix all warnings with ``[needs]``, and allow them to be suppressed (`#975 <https://github.com/useblocks/sphinx-needs/pull/975>`_)

1.3.0
-----
Released: 16.08.2023
Expand Down
19 changes: 14 additions & 5 deletions sphinx_needs/api/need.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ def run():
if need_type not in configured_need_types:
logger.warning(
"Couldn't create need {}. Reason: The need-type (i.e. `{}`) is not set "
"in the project's 'need_types' configuration in conf.py.".format(id, need_type)
"in the project's 'need_types' configuration in conf.py. [needs]".format(id, need_type),
type="needs",
)

for ntype in types:
Expand Down Expand Up @@ -216,7 +217,10 @@ def run():
new_tags = [] # Shall contain only valid tags
for i in range(len(tags)):
if len(tags[i]) == 0 or tags[i].isspace():
logger.warning(f"Scruffy tag definition found in need {need_id}. " "Defined tag contains spaces only.")
logger.warning(
f"Scruffy tag definition found in need {need_id}. " "Defined tag contains spaces only. [needs]",
type="needs",
)
else:
new_tags.append(tags[i])

Expand Down Expand Up @@ -245,7 +249,9 @@ def run():
for i in range(len(constraints)):
if len(constraints[i]) == 0 or constraints[i].isspace():
logger.warning(
f"Scruffy tag definition found in need {need_id}. " "Defined constraint contains spaces only."
f"Scruffy tag definition found in need {need_id}. "
"Defined constraint contains spaces only. [needs]",
type="needs",
)
else:
new_constraints.append(constraints[i])
Expand Down Expand Up @@ -506,7 +512,7 @@ def del_need(app: Sphinx, need_id: str) -> None:
if need_id in env.needs_all_needs:
del env.needs_all_needs[need_id]
else:
logger.warning(f"Given need id {need_id} not exists!")
logger.warning(f"Given need id {need_id} not exists! [needs]", type="needs")


def add_external_need(
Expand Down Expand Up @@ -621,7 +627,10 @@ def _read_in_links(links_string: Union[str, List[str]]) -> List[str]:
link_list = links_string
for link in link_list:
if link.isspace():
logger.warning(f"Grubby link definition found in need {id}. " "Defined link contains spaces only.")
logger.warning(
f"Grubby link definition found in need {id}. " "Defined link contains spaces only. [needs]",
type="needs",
)
else:
links.append(link.strip())

Expand Down
5 changes: 4 additions & 1 deletion sphinx_needs/diagrams_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ def collect_diagram_attributes(self) -> Dict[str, Any]:
if len(link_types[i]) == 0 or link_types[i].isspace():
del link_types[i]
logger.warning(
"Scruffy link_type definition found in needsequence. " "Defined link_type contains spaces only."
"Scruffy link_type definition found in needsequence. "
"Defined link_type contains spaces only. [needs]",
type="needs",
location=(env.docname, self.lineno),
)

config_names = self.options.get("config")
Expand Down
10 changes: 7 additions & 3 deletions sphinx_needs/directives/need.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ def read_in_links(self, name: str) -> List[str]:
if link.isspace():
logger.warning(
f"Grubby link definition found in need '{self.trimmed_title}'. "
"Defined link contains spaces only."
"Defined link contains spaces only. [needs]",
type="needs",
location=(self.env.docname, self.lineno),
)
else:
links.append(link.strip())
Expand Down Expand Up @@ -206,8 +208,10 @@ def _get_full_title(self) -> str:
if len(self.arguments) > 0: # a title was passed
if "title_from_content" in self.options:
self.log.warning(
'Needs: need "{}" has :title_from_content: set, '
"but a title was provided. (see file {})".format(self.arguments[0], self.docname)
'need "{}" has :title_from_content: set, '
"but a title was provided. (see file {}) [needs]".format(self.arguments[0], self.docname),
type="needs",
location=(self.env.docname, self.lineno),
)
return self.arguments[0]
elif self.title_from_content:
Expand Down
17 changes: 12 additions & 5 deletions sphinx_needs/directives/needflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ def run(self) -> Sequence[nodes.Node]:
targetnode = nodes.target("", "", ids=[targetid])

all_link_types = ",".join(x["option"] for x in env.config.needs_extra_links)
link_types = list(split_link_types(self.options.get("link_types", all_link_types)))
link_types = list(
split_link_types(self.options.get("link_types", all_link_types), location=(env.docname, self.lineno))
)

config_names = self.options.get("config")
configs = []
Expand Down Expand Up @@ -110,10 +112,14 @@ def run(self) -> Sequence[nodes.Node]:
return [targetnode, Needflow("")]


def split_link_types(link_types: str) -> Iterable[str]:
def split_link_types(link_types: str, location=None) -> Iterable[str]:
def is_valid(link_type: str) -> bool:
if len(link_type) == 0 or link_type.isspace():
logger.warning("Scruffy link_type definition found in needflow." "Defined link_type contains spaces only.")
logger.warning(
"Scruffy link_type definition found in needflow." "Defined link_type contains spaces only. [needs]",
type="needs",
location=location,
)
return False
return True

Expand Down Expand Up @@ -309,9 +315,10 @@ def process_needflow(app: Sphinx, doctree: nodes.document, fromdocname: str, fou
for lt in option_link_types:
if lt not in [link["option"].upper() for link in link_types]:
logger.warning(
"Unknown link type {link_type} in needflow {flow}. Allowed values: {link_types}".format(
"Unknown link type {link_type} in needflow {flow}. Allowed values: {link_types} [needs]".format(
link_type=lt, flow=current_needflow["target_id"], link_types=",".join(link_types)
)
),
type="needs",
)

content = []
Expand Down
3 changes: 2 additions & 1 deletion sphinx_needs/directives/needgantt.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ def process_needgantt(app, doctree, fromdocname, found_nodes):
if not (duration and duration.isdigit()):
logger.warning(
"Duration not set or invalid for needgantt chart. "
"Need: {}. Duration: {}".format(need["id"], duration)
"Need: {}. Duration: {} [needs]".format(need["id"], duration),
type="needs",
)
duration = 1
gantt_element = "[{}] as [{}] lasts {} days\n".format(need["title"], need["id"], duration)
Expand Down
10 changes: 7 additions & 3 deletions sphinx_needs/directives/needimport.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ def run(self) -> Sequence[nodes.Node]:
logger.warning(
"Deprecation warning: Relative path must be relative to the current document in future, "
"not to the conf.py location. Use a starting '/', like '/needs.json', to make the path "
"relative to conf.py."
"relative to conf.py. [needs]",
type="needs",
location=(self.env.docname, self.lineno),
)
else:
# Absolute path starts with /, based on the source directory. The / need to be striped
Expand Down Expand Up @@ -140,9 +142,11 @@ def run(self) -> Sequence[nodes.Node]:
needs_list_filtered[key] = need
except Exception as e:
logger.warning(
"needimport: Filter {} not valid. Error: {}. {}{}".format(
"needimport: Filter {} not valid. Error: {}. {}{} [needs]".format(
filter_string, e, self.docname, self.lineno
)
),
type="needs",
location=(self.env.docname, self.lineno),
)

needs_list = needs_list_filtered
Expand Down
5 changes: 3 additions & 2 deletions sphinx_needs/directives/needsequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ def process_needsequence(app: Sphinx, doctree: nodes.document, fromdocname: str,
if lt not in [link["option"].upper() for link in link_types]:
logger.warning(
"Unknown link type {link_type} in needsequence {flow}. Allowed values:"
" {link_types}".format(
" {link_types} [needs]".format(
link_type=lt, flow=current_needsequence["target_id"], link_types=",".join(link_types)
)
),
type="needs",
)

content = []
Expand Down
2 changes: 1 addition & 1 deletion sphinx_needs/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def find_css_files() -> Iterable[Path]:

if not source_file_path.exists():
source_file_path = css_root / "blank" / "blank.css"
logger.warning(f"{source_file_path} not found. Copying sphinx-internal blank.css")
logger.warning(f"{source_file_path} not found. Copying sphinx-internal blank.css [needs]", type="needs")

dest_file = dest_dir / source_file_path.name
dest_dir.mkdir(exist_ok=True)
Expand Down
7 changes: 4 additions & 3 deletions sphinx_needs/filter_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def process_filters(app: Sphinx, all_needs, current_needlist, include_external:
try:
all_needs = sorted(all_needs, key=lambda node: node[sort_key] or "")
except KeyError as e:
log.warning(f"Sorting parameter {sort_key} not valid: Error: {e}")
log.warning(f"Sorting parameter {sort_key} not valid: Error: {e} [needs]", type="needs")

# check if include external needs
checked_all_needs = []
Expand Down Expand Up @@ -166,7 +166,7 @@ def process_filters(app: Sphinx, all_needs, current_needlist, include_external:
filter_func = measure_time(category="filter_func", source="user", func=filter_func)
filter_func(**context)
else:
log.warning("Something went wrong running filter")
log.warning("Something went wrong running filter [needs]", type="needs")
return []

# The filter results may be dirty, as it may continue manipulated needs.
Expand Down Expand Up @@ -268,7 +268,8 @@ def filter_needs(app: Sphinx, needs, filter_string: str = "", current_need=None)
found_needs.append(filter_need)
except Exception as e:
if not error_reported: # Let's report a filter-problem only onces
log.warning(e)
location = (current_need["docname"], current_need["lineno"]) if current_need else None
log.warning(str(e) + " [needs]", type="needs", location=location)
error_reported = True

return found_needs
Expand Down
4 changes: 2 additions & 2 deletions sphinx_needs/functions/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ def check_linked_values(
if not filter_single_need(app, needs[link], filter_string):
continue
except Exception as e:
logger.warning(f"CheckLinkedValues: Filter {filter_string} not valid: Error: {e}")
logger.warning(f"CheckLinkedValues: Filter {filter_string} not valid: Error: {e} [needs]", type="needs")

if not one_hit and needs[link][search_option] not in search_value:
return None
Expand Down Expand Up @@ -409,7 +409,7 @@ def calc_sum(app: Sphinx, need, needs, option, filter=None, links_only: bool = F
except ValueError:
pass
except NeedsInvalidFilter as ex:
logger.warning(f"Given filter is not valid. Error: {ex}")
logger.warning(f"Given filter is not valid. Error: {ex} [needs]", type="needs")

with contextlib.suppress(ValueError):
calculated_sum += float(check_need[option])
Expand Down
4 changes: 3 additions & 1 deletion sphinx_needs/need_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ def process_constraints(app: Sphinx, need: Dict[str, Any]) -> None:

if "warn" in actions_on_fail:
logger.warning(
f"Constraint {cmd} for need {need_id} FAILED! severity: {severity}", color="red"
f"Constraint {cmd} for need {need_id} FAILED! severity: {severity} [needs]",
type="needs",
color="red",
)

if "break" in actions_on_fail:
Expand Down
4 changes: 2 additions & 2 deletions sphinx_needs/needs.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ def load_config(app: Sphinx, *_args) -> None:
existing_extra_options = NEEDS_CONFIG.get("extra_options")
for option in app.config.needs_extra_options:
if option in existing_extra_options:
log.warning(f'extra_option "{option}" already registered.')
log.warning(f'extra_option "{option}" already registered. [needs]', type="needs")
NEEDS_CONFIG.add("extra_options", {option: directives.unchanged}, dict, True)
extra_options = NEEDS_CONFIG.get("extra_options")

Expand Down Expand Up @@ -524,7 +524,7 @@ def load_config(app: Sphinx, *_args) -> None:
if name not in existing_warnings:
NEEDS_CONFIG.add("warnings", {name: check}, dict, append=True)
else:
log.warning(f'{name} for "warnings" is already registered.')
log.warning(f'{name} for "warnings" is already registered. [needs]', type="needs")


def visitor_dummy(*_args, **_kwargs) -> None:
Expand Down
4 changes: 2 additions & 2 deletions sphinx_needs/needsfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def load_json(self, file) -> None:
file = os.path.join(self.confdir, file)

if not os.path.exists(file):
self.log.warning(f"Could not load needs json file {file}")
self.log.warning(f"Could not load needs json file {file} [needs]", type="needs")
else:
errors = check_needs_file(file)
# We only care for schema errors here, all other possible errors
Expand All @@ -125,7 +125,7 @@ def load_json(self, file) -> None:
try:
needs_list = json.loads(needs_file_content)
except json.JSONDecodeError:
self.log.warning(f"Could not decode json file {file}")
self.log.warning(f"Could not decode json file {file} [needs]", type="needs")
else:
self.needs_list = needs_list

Expand Down
2 changes: 1 addition & 1 deletion sphinx_needs/roles/need_incoming.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def process_need_incoming(app: Sphinx, doctree: nodes.document, fromdocname: str
pass

else:
env.warn_node("Needs: need %s not found" % node_need_backref["reftarget"], node_need_backref)
env.warn_node("need %s not found [needs]" % node_need_backref["reftarget"], node_need_backref)

if len(node_link_container.children) == 0:
node_link_container += nodes.Text("None")
Expand Down
12 changes: 8 additions & 4 deletions sphinx_needs/roles/need_outgoing.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,25 +117,29 @@ def process_need_outgoing(
and extra_links_dict[link_type]["allow_dead_links"]
):
log_level = "INFO"
kwargs = {}
else:
# Set an extra css class, if link type is not configured to allow dead links
dead_link_para.attributes["classes"].append("forbidden")
log_level = "WARNING"
kwargs = {"type": "needs"}

if report_dead_links:
if node_need_ref and node_need_ref.line:
log.log(
log_level,
f"Needs: linked need {link} not found "
f"(Line {node_need_ref.line} of file {node_need_ref.source})",
f"linked need {link} not found "
f"(Line {node_need_ref.line} of file {node_need_ref.source}) [needs]",
**kwargs,
)
else:
log.log(
log_level,
"Needs: outgoing linked need {} not found (document: {}, "
"source need {} on line {} )".format(
"outgoing linked need {} not found (document: {}, "
"source need {} on line {} ) [needs]".format(
link, ref_need["docname"], ref_need["id"], ref_need["lineno"]
),
**kwargs,
)

# If we have several links, we add an empty text between them
Expand Down
5 changes: 3 additions & 2 deletions sphinx_needs/roles/need_part.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ def update_need_with_parts(env: BuildEnvironment, need, part_nodes: List[NeedPar

if inline_id in need["parts"]:
log.warning(
"part_need id {} in need {} is already taken. need_part may get overridden.".format(
"part_need id {} in need {} is already taken. need_part may get overridden. [needs]".format(
inline_id, need["id"]
)
),
type="needs",
)

need["parts"][inline_id] = {
Expand Down
14 changes: 7 additions & 7 deletions sphinx_needs/roles/need_ref.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ def process_need_ref(app: Sphinx, doctree: nodes.document, fromdocname: str, fou
try:
link_text = ref_name.format(**dict_need)
except KeyError as e:
link_text = '"Needs: option placeholder %s for need %s not found (Line %i of file %s)"' % (
link_text = '"option placeholder %s for need %s not found (Line %i of file %s)"' % (
e,
node_need_ref["reftarget"],
node_need_ref.line,
node_need_ref.source,
)
log.warning(link_text)
log.warning(link_text + " [needs]", type="needs")
else:
if ref_name:
# If ref_name differs from the need id, we treat the "ref_name content" as title.
Expand All @@ -123,10 +123,9 @@ def process_need_ref(app: Sphinx, doctree: nodes.document, fromdocname: str, fou
link_text = app.config.needs_role_need_template.format(**dict_need)
except KeyError as e:
link_text = (
'"Needs: the config parameter needs_role_need_template uses not supported placeholders: %s "'
% e
'"the config parameter needs_role_need_template uses not supported placeholders: %s "' % e
)
log.warning(link_text)
log.warning(link_text + " [needs]", type="needs")

node_need_ref[0].children[0] = nodes.Text(link_text)

Expand All @@ -147,8 +146,9 @@ def process_need_ref(app: Sphinx, doctree: nodes.document, fromdocname: str, fou

else:
log.warning(
"Needs: linked need %s not found (Line %i of file %s)"
% (node_need_ref["reftarget"], node_need_ref.line, node_need_ref.source)
"linked need %s not found (Line %i of file %s) [needs]"
% (node_need_ref["reftarget"], node_need_ref.line, node_need_ref.source),
type="needs",
)

node_need_ref.replace_self(new_node_ref)
Loading

0 comments on commit 5b983a4

Please sign in to comment.