[hawkmoth] Re: [PATCH v2 2/3] hawkmoth: add support for propagating diagnostics

  • From: Bruno Santos <brunomanuelsantos@xxxxxxxxxxxxxxxxxx>
  • To: hawkmoth@xxxxxxxxxxxxx
  • Date: Wed, 1 May 2019 14:01:59 +0200

On 12:21:20 2019-05-01, Marius Vlad wrote:

Thank so much for taking care of these! 

Have a few of small questions bellow:

On Wed, May 01, 2019 at 12:28:37AM +0200, Bruno Santos wrote:
At the same time, the first diagnostics are implemented or rather
forwarded from libclang parse.

Original concept: Marius Vlad
---
 hawkmoth/__init__.py  | 22 ++++++++++++++++++++--
 hawkmoth/__main__.py  |  6 +++++-
 hawkmoth/parser.py    | 27 ++++++++++++++++++++++++++-
 test/test_hawkmoth.py |  2 +-
 4 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/hawkmoth/__init__.py b/hawkmoth/__init__.py
index 3355922..0896453 100644
--- a/hawkmoth/__init__.py
+++ b/hawkmoth/__init__.py
@@ -21,7 +21,7 @@
 from sphinx.util.docutils import switch_source_input
 from sphinx.util import logging
 
-from hawkmoth.parser import parse
+from hawkmoth.parser import parse, ErrorLevel
 
 with open(os.path.join(os.path.abspath(os.path.dirname(__file__)),
                        'VERSION')) as version_file:
@@ -42,13 +42,31 @@ class CAutoDocDirective(Directive):
     }
     has_content = False
 
+    # Map verbosity levels to logger levels.
+    _log_lvl = {ErrorLevel.ERROR: logging.LEVEL_NAMES['ERROR'],
+                ErrorLevel.WARNING: logging.LEVEL_NAMES['WARNING'],
+                ErrorLevel.INFO: logging.LEVEL_NAMES['INFO'],
There's no ErrorLevel.INFO in that parser Enum. Do we care about it?

Hey,

INFO is there. Am I missing something?


+                ErrorLevel.DEBUG: logging.LEVEL_NAMES['DEBUG']}
+
+    def __display_parser_diagnostics(self, errors):
+        env = self.state.document.settings.env
+
+        for (severity, filename, lineno, msg) in errors:
+            toprint = '{}:{}: {}'.format(filename, lineno, msg)
+
+            if severity.value <= env.app.verbosity:
This should be strict such that in case there's no verbosity (0) it
doesn't match of equal to 0 severity value of ERROR.

This is intentional. Errors should always be shown, warnings must be
enabled and further levels require further flags. Seems reasonable to
me.

Point of reference, calling gcc always shows errors, it's the warnings
that start out disabled by default.

+                self.logger.log(self._log_lvl[severity], toprint,
+                                location=(env.docname, self.lineno))
+
     def __parse(self, viewlist, filename):
         env = self.state.document.settings.env
 
         compat = self.options.get('compat', env.config.cautodoc_compat)
         clang = self.options.get('clang', env.config.cautodoc_clang)
 
-        comments = parse(filename, compat=compat, clang=clang)
+        comments, errors = parse(filename, compat=compat, clang=clang)
+
+        self.__display_parser_diagnostics(errors)
 
         for (comment, meta) in comments:
             lineoffset = meta['line'] - 1
diff --git a/hawkmoth/__main__.py b/hawkmoth/__main__.py
index 1271772..1efb267 100644
--- a/hawkmoth/__main__.py
+++ b/hawkmoth/__main__.py
@@ -31,11 +31,15 @@ def main():
                         help='Verbose output.')
     args = parser.parse_args()
 
-    docs = parse(args.file, compat=args.compat, clang=args.clang)
+    docs, errors = parse(args.file, compat=args.compat, clang=args.clang)
 
     for (doc, meta) in docs:
         if args.verbose:
             print('# {}'.format(meta))
         print(doc)
 
+    for (severity, filename, lineno, msg) in errors:
+        print('{}: {}:{}: {}\n'.format(severity.name,
+                                       filename, lineno, msg), 
file=sys.stderr)
+
 main()
diff --git a/hawkmoth/parser.py b/hawkmoth/parser.py
index ecb0d24..60002bc 100644
--- a/hawkmoth/parser.py
+++ b/hawkmoth/parser.py
@@ -33,6 +33,7 @@
 Otherwise, documentation comments are passed through verbatim.
 """
 
+import enum
 import itertools
 import sys
 
@@ -43,6 +44,16 @@
 
 from hawkmoth.util import docstr, doccompat
 
+class ErrorLevel(enum.Enum):
+    """
+    Supported error levels in inverse numerical order of severity. The 
values
+    are chosen so that they map directly to a 'verbosity level'.
+    """
+    ERROR = 0
+    WARNING = 1
+    INFO = 2
+    DEBUG = 3

Is there a reason not to include all? (FATAL/NOTE)...

The current proposal is:

- ERROR: Something went terribly wrong, documentation is likely broken.

- WARNING: May have affected the documentation output in subtle ways.
  Maybe some symbol is not fully defined and therefore won't be linked.
  Documentation is still sound though.

- INFO: Tangentially related to documentation, but of no consequence to
  the output. E.g. 'entering file xxx', 'found function token', etc.

- DEBUG: Don't affect documentation, but may be important in inspecting
  what the code is doing for debug/development purposes. E.g. some
  lesser error from an external module that we discarded.

I don't see what FATAL and NOTE would bring to the table.

If we really get a lot more errors and must discriminate further, I'd
argue for an horizontal expansion rather than vertical: (e.g.) -wclang,
etc. Adding more levels won't really help in my opinion.

+
 def comment_extract(tu):
 
     # FIXME: How to handle top level comments above a cursor that it does 
*not*
@@ -239,10 +250,22 @@ def _recursive_parse(comments, cursor, nest, compat):
 
     return [(doc, meta)]
 
+def clang_diagnostics(errors, diagnostics):
+    sev = {0: ErrorLevel.DEBUG,
+           1: ErrorLevel.DEBUG,
+           2: ErrorLevel.WARNING,
+           3: ErrorLevel.ERROR,
+           4: ErrorLevel.ERROR}
I suppose the same applies here? 4 and 3 are fatal, respective error.

See previous answer.

Cheers

+    for diag in diagnostics:
+        errors.extend([(sev[diag.severity], diag.location.file.name,
+                        diag.location.line, diag.spelling)])
+
 # return a list of (comment, metadata) tuples
 # options - dictionary with directive options
 def parse(filename, **options):
 
+    errors = []
     args = options.get('clang')
     if args is not None:
         args = [s.strip() for s in args.split(',') if len(s.strip()) > 0]
@@ -255,6 +278,8 @@ def parse(filename, **options):
                      TranslationUnit.PARSE_DETAILED_PROCESSING_RECORD |
                      TranslationUnit.PARSE_SKIP_FUNCTION_BODIES)
 
+    clang_diagnostics(errors, tu.diagnostics)
+
     top_level_comments, comments = comment_extract(tu)
 
     result = []
@@ -270,4 +295,4 @@ def parse(filename, **options):
     # Sort all elements by order of appearance.
     result.sort(key=lambda r: r[1]['line'])
 
-    return result
+    return result, errors
diff --git a/test/test_hawkmoth.py b/test/test_hawkmoth.py
index 07c2c14..aa68a9c 100755
--- a/test/test_hawkmoth.py
+++ b/test/test_hawkmoth.py
@@ -11,7 +11,7 @@
 def _get_output(input_filename, **options):
     docs_str = ''
 
-    docs = parse(input_filename, **options)
+    docs, errors = parse(input_filename, **options)
 
     for (doc, meta) in docs:
         docs_str += doc + '\n'
-- 
2.21.0




Attachment: signature.asc
Description: PGP signature

Other related posts:

  1. Home
  2. hawkmoth
  3. Archive
  4. 05-2019