Problem reporting revamp, program is now aware of its problem types

Sat, 08 Jun 2019 01:32:25 +0300

author
Teemu Piippo <teemu@hecknology.net>
date
Sat, 08 Jun 2019 01:32:25 +0300
changeset 62
f0a6bf48b05e
parent 61
15c95d3fcfd8
child 63
8949af6a4279

Problem reporting revamp, program is now aware of its problem types

geometry.py file | annotate | diff | comparison | revisions
ldcheck.py file | annotate | diff | comparison | revisions
parse.py file | annotate | diff | comparison | revisions
static/warning.svg file | annotate | diff | comparison | revisions
templates/webfront.html file | annotate | diff | comparison | revisions
tests/misc.py file | annotate | diff | comparison | revisions
tests/quadrilaterals.py file | annotate | diff | comparison | revisions
tests/subfiles.py file | annotate | diff | comparison | revisions
testsuite.py file | annotate | diff | comparison | revisions
webfront.py file | annotate | diff | comparison | revisions
--- a/geometry.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/geometry.py	Sat Jun 08 01:32:25 2019 +0300
@@ -1,6 +1,9 @@
 def degree_rep(angle):
     from math import degrees
-    return '%.2f°' % degrees(angle)
+    try:
+        return '%.2f°' % degrees(angle)
+    except TypeError:
+        return angle
 
 def position_vector(vertex):
     assert isinstance(vertex, Vertex)
--- a/ldcheck.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/ldcheck.py	Sat Jun 08 01:32:25 2019 +0300
@@ -63,18 +63,33 @@
         ]
 
 import argparse
+
+def default_problem_message(message):
+    if callable(message):
+        import inspect
+        spec = inspect.getfullargspec(message)
+        args = {}
+        assert not spec.varargs and not spec.varkw
+        for argname in spec.args + spec.kwonlyargs:
+            args[argname] = '<' + argname.replace('_', ' ') + '>'
+        return message(**args)
+    else:
+        return message
+
 class ListTestSuiteAction(argparse.Action):
     def __init__(self, option_strings, dest, nargs = None, **kwargs):
         super().__init__(option_strings, dest, nargs = 0, **kwargs)
     def __call__(self, *args, **kwargs):
-        from testsuite import load_tests
+        from testsuite import load_tests, all_warning_types
         from sys import exit
         from re import sub
         test_suite = load_tests()
-        for test_name in sorted(test_suite['tests'].keys()):
-            test_function = test_suite['tests'][test_name]
-            help = sub(r'\s+', ' ', test_function.__doc__ or '').strip()
-            print(test_name + ': ' + help)
+        for warning_type in sorted(all_warning_types(test_suite), key = lambda k: k.name):
+            print(str.format('{name}: {severity}: "{message}"',
+                name = warning_type.name,
+                severity = warning_type.severity,
+                message = default_problem_message(warning_type.message),
+            ))
         exit(0)
 
 if __name__ == '__main__':
--- a/parse.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/parse.py	Sat Jun 08 01:32:25 2019 +0300
@@ -2,7 +2,6 @@
 import re
 from geometry import *
 from colours import Colour
-from testsuite import error
 import header
 
 class BadLdrawLine(Exception):
--- a/static/warning.svg	Wed Jun 05 00:33:50 2019 +0300
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,103 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<svg
-   xmlns:dc="http://purl.org/dc/elements/1.1/"
-   xmlns:cc="http://creativecommons.org/ns#"
-   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
-   xmlns:svg="http://www.w3.org/2000/svg"
-   xmlns="http://www.w3.org/2000/svg"
-   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
-   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
-   width="48"
-   height="48"
-   version="1.1"
-   id="svg998"
-   sodipodi:docname="warning.svg"
-   inkscape:version="0.92.2 (5c3e80d, 2017-08-06)">
-  <metadata
-     id="metadata1002">
-    <rdf:RDF>
-      <cc:Work
-         rdf:about="">
-        <dc:format>image/svg+xml</dc:format>
-        <dc:type
-           rdf:resource="http://purl.org/dc/dcmitype/StillImage" />
-      </cc:Work>
-    </rdf:RDF>
-  </metadata>
-  <sodipodi:namedview
-     pagecolor="#ffffff"
-     bordercolor="#666666"
-     borderopacity="1"
-     objecttolerance="10"
-     gridtolerance="10"
-     guidetolerance="10"
-     inkscape:pageopacity="0"
-     inkscape:pageshadow="2"
-     inkscape:window-width="1920"
-     inkscape:window-height="1049"
-     id="namedview1000"
-     showgrid="false"
-     inkscape:zoom="3.1267378"
-     inkscape:cx="-74.508859"
-     inkscape:cy="4.4248979"
-     inkscape:window-x="1920"
-     inkscape:window-y="0"
-     inkscape:window-maximized="1"
-     inkscape:current-layer="svg998" />
-  <defs
-     id="defs982">
-    <radialGradient
-       id="a"
-       gradientUnits="userSpaceOnUse"
-       cy="41.4"
-       cx="24.29"
-       gradientTransform="matrix(1 0 0 .3526 0 26.8)"
-       r="21.1">
-      <stop
-         offset="0"
-         id="stop977" />
-      <stop
-         stop-opacity="0"
-         offset="1"
-         id="stop979" />
-    </radialGradient>
-  </defs>
-  <ellipse
-     opacity=".41"
-     rx="21.1"
-     ry="7.44"
-     cy="41.4"
-     cx="24.29"
-     fill="url(#a)"
-     id="ellipse984" />
-  <circle
-     cy="21.4"
-     cx="24.3"
-     r="21.27"
-     fill="#914900"
-     id="circle986" />
-  <circle
-     r="20"
-     cy="21.31"
-     stroke="#fcaf3e"
-     cx="24.44"
-     fill="#f57900"
-     id="circle988"
-     style="fill:#f5ca00;fill-opacity:1" />
-  <path
-     style="fill:#000000;fill-opacity:1"
-     inkscape:connector-curvature="0"
-     id="path990"
-     d="m 21.46,10.39 c -0.13,0 -0.23,0.16 -0.23,0.37 L 22.3,25.2 c 0,0.21 0.11,0.38 0.23,0.38 h 3.57 c 0.13,0 0.23,-0.17 0.23,-0.38 L 27.4,10.76 c 0,-0.21 -0.11,-0.37 -0.23,-0.37 h -5.7" />
-  <path
-     style="fill:#ffffff;fill-opacity:0.21000001"
-     inkscape:connector-curvature="0"
-     id="path992"
-     d="m 43.68,20.48 c 0,10.83 -6.1,-4.31 -18.67,0.39 C 12.28,25.59 4.43,31.31 4.43,20.48 4.43,9.65 13.23,0.86 24.06,0.86 c 10.83,0 19.62,8.79 19.62,19.62" />
-  <circle
-     style="fill:#000000;fill-opacity:1"
-     id="circle994"
-     r="2.3"
-     cx="24.360001"
-     cy="30.200001" />
-</svg>
--- a/templates/webfront.html	Wed Jun 05 00:33:50 2019 +0300
+++ b/templates/webfront.html	Sat Jun 08 01:32:25 2019 +0300
@@ -20,15 +20,15 @@
     {% if report['problems'] %}
         <ul class="problems-list">
         {% for problem in report['problems'] %}
-            <li class="problem-{{problem['type']}}">
+            <li class="problem-{{problem['severity']}}">
                 <img
-                    src="static/{{problem['type']}}.svg"
+                    src="static/{{problem['severity']}}.svg"
                     width="32" height="32"
                     class="problem-icon"
                 />
-                Line {{problem['line-number']}}: {{problem['message']}}
+                Line {{problem.line_number}}: {{problem.message_str}}
                 <br />
-                <span class="ldraw-code">{{problem['ldraw-code']}}</span>
+                <span class="ldraw-code">{{problem.ldraw_code}}</span>
             </li>
         {% endfor %}
         </ul>
--- a/tests/misc.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/tests/misc.py	Sat Jun 08 01:32:25 2019 +0300
@@ -1,83 +1,114 @@
-from testsuite import error, warning
+from testsuite import problem_type, report_problem
 import linetypes
 
+@problem_type('bad-colour',
+    severity = 'error',
+    message = lambda colour_index, count: str.format(
+        'invalid colour {} used {} time(s)',
+        colour_index,
+        count,
+    ),
+)
 def colours_test(model):
     ''' Checks that all colours used in the part model are valid. '''
-    yield from (
-        warning(element, 'bad-colour', colour_index = element.colour.index)
-        for element in model.body
-        if hasattr(element, 'colour') and not element.colour.is_valid
-    ) 
+    from collections import defaultdict
+    bad_colours = defaultdict(lambda: {'count': 0, 'first-occurrence': None})
+    for element in model.body:
+        if hasattr(element, 'colour') and not element.colour.is_valid:
+            bad_colours[element.colour.index]['count'] += 1
+            if not bad_colours[element.colour.index]['first-occurrence']:
+                bad_colours[element.colour.index]['first-occurrence'] = element
+    yield from [
+        report_problem(
+            'bad-colour',
+            bad_object = bad_colour['first-occurrence'],
+            colour_index = colour_index,
+            count = bad_colour['count'],
+        )
+        for colour_index, bad_colour in bad_colours.items()
+    ]
 
+@problem_type('syntax-error',
+    severity = 'error',
+    message = lambda reason: str.format('syntax error: {}', reason),
+)
 def syntax_errors(model):
     yield from (
-        error(element, 'syntax-error', reason = element.reason)
+        report_problem('syntax-error',
+            bad_object = element,
+            reason = element.reason
+        )
         for element in model.body
         if isinstance(element, linetypes.Error)
     )
 
+@problem_type('bad-header',
+    severity = 'error',
+    message = lambda reason: str.format('bad header: {}', reason),
+)
 def bad_header(model):
     import header
     if isinstance(model.header, header.BadHeader):
-        yield error(
-            model.body[model.header.index],
+        yield report_problem(
             'bad-header',
+            bad_object = model.body[model.header.index],
             reason = model.header.reason,
         )
 
+@problem_type('bfc-nocertify',
+    severity = 'error',
+    message = 'all new parts must be BFC certified',
+)
 def nocertify_test(model):
     import header
     if model.header.valid and model.header.bfc == 'NOCERTIFY':
-        yield error(
-            model.body[model.header.first_occurrence['bfc']],
-            'bfc-nocertify')
+        yield report_problem(
+            'bfc-nocertify',
+            bad_object = model.body[model.header.first_occurrence['bfc']],
+        )
 
+@problem_type('physical-colour-part',
+    severity = 'error',
+    message = 'no new physical colour parts are accepted',
+)
 def physical_colours_test(model):
     if model.header.valid and 'Physical_Colour' in model.header.qualifiers:
-        yield error(
-            model.body[model.header.first_occurrence['part type']],
-            'physical-colour')
+        yield report_problem(
+            'physical-colour-part',
+            bad_object = model.body[model.header.first_occurrence['part type']],
+        )
 
+@problem_type('official-part',
+    severity = 'error',
+    message = 'new parts must be unofficial',
+)
 def unofficiality_test(model):
     if model.header.valid and not model.header.filetype.startswith('Unofficial_'):
-        yield error(
-            model.body[model.header.first_occurrence['part type']],
-            'unofficial-type')
+        yield report_problem(
+            'unofficial-part',
+            bad_object = model.body[model.header.first_occurrence['part type']])
 
+@problem_type('primitive-ccw',
+    severity = 'error',
+    message = 'primitives must have CCW winding',
+)
 def primitive_ccw_test(model):
     if model.header.valid \
     and model.header.filetype.endswith('Primitive') \
     and model.header.bfc != 'CERTIFY CCW':
-        yield error(
-            model.body[model.header.first_occurrence['bfc']],
-            'primitive-bfc-ccw')
+        yield report_problem(
+            'primitive-bfc-ccw',
+            bad_object = model.body[model.header.first_occurrence['bfc']],
+        )
 
 manifest = {
-    'tests': {
-        'colour-validity': colours_test,
-        'syntax-errors': syntax_errors,
-        'header-validity': bad_header,
-        'bfc-nocertify': nocertify_test,
-        'physical-colour': physical_colours_test,
-        'unofficial-type': unofficiality_test,
-        'primitive-ccw': primitive_ccw_test,
-    },
-    'messages': {
-        'bad-colour': lambda colour_index: str.format(
-            'invalid colour {}',
-            colour_index,
-        ),
-        'syntax-error': lambda reason: str.format(
-            'syntax error: {}',
-            reason,
-        ),
-        'bad-header': lambda reason: str.format(
-            'bad header: {}',
-            reason,
-        ),
-        'bfc-nocertify': 'all new parts must be BFC certified',
-        'physical-colour': 'no new physical colour parts are accepted',
-        'unofficial-type': 'new parts must be unofficial',
-        'primitive-bfc-ccw': 'primitives must have CCW winding',
-    },
+    'tests': [
+        colours_test,
+        syntax_errors,
+        bad_header,
+        nocertify_test,
+        physical_colours_test,
+        unofficiality_test,
+        primitive_ccw_test,
+    ],
 }
--- a/tests/quadrilaterals.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/tests/quadrilaterals.py	Sat Jun 08 01:32:25 2019 +0300
@@ -1,11 +1,12 @@
 from math import radians
-from testsuite import warning, error, notice
+from testsuite import problem_type, report_problem
 from geometry import *
 
 def sign_consistency(container):
     # Returns whether all elements in container have the same sign
     return min(container) * max(container) >= 0
 
+@problem_type('concave', severity = 'error', message = 'concave quadrilateral')
 def concave_test(model):
     ''' Checks that all quadrilaterals are convex. '''
     for quadrilateral in model.quadrilaterals:
@@ -19,8 +20,22 @@
             for v1, v2, v3 in pairs(geometry.vertices, count = 3)
         ]
         if not sign_consistency(z_scores):
-            yield error(quadrilateral, 'concave-error')
+            yield report_problem('concave', bad_object = quadrilateral)
 
+@problem_type('skew-major',
+    severity = 'error',
+    message = lambda skew_angle:
+        str.format('skew quadrilateral (plane angle {})',
+            degree_rep(skew_angle),
+        ),
+)
+@problem_type('skew-minor',
+    severity = 'notice',
+    message = lambda skew_angle:
+        str.format('slightly skew quadrilateral (plane angle {})',
+            degree_rep(skew_angle),
+        ),
+)
 def skew_test(model):
     ''' Checks that all quadrilaterals are coplanar. '''
     for quadrilateral in model.quadrilaterals:
@@ -29,16 +44,24 @@
             plane_2 = triangle_plane_normal(triangles[1])
             skew_angle = vector_angle(plane_1, plane_2, normalized = True)
             if skew_angle > radians(3.0):
-                yield error(quadrilateral, 'skew-error',
+                yield report_problem(
+                    'skew-major',
+                    bad_object = quadrilateral,
                     skew_angle = skew_angle,
                 )
                 break
             elif skew_angle > radians(1.0):
-                yield warning(quadrilateral, 'skew-warning',
+                yield report_problem(
+                    'skew-minor',
+                    bad_object = quadrilateral,
                     skew_angle = skew_angle,
                 )
                 break
 
+@problem_type('self-intersecting',
+    severity = 'error',
+    message = 'self-intersecting quadrilateral',
+)
 def bowtie_test(model):
     for quadrilateral in model.quadrilaterals:
         geometry = transform_to_xy(quadrilateral.geometry)
@@ -48,52 +71,44 @@
             line_2 = LineSegment(vertices[2 + i], vertices[3 + i])
             intersection = line_segment_intersection_xy(line_1, line_2)
             if intersection:
-                yield error(quadrilateral, 'self-intersecting')
+                yield report_problem(
+                    'self-intersecting',
+                    bad_object = quadrilateral,
+                )
                 break
 
+@problem_type('collinear', severity = 'error', message = 'collinear polygon')
 def collinear_test(model):
     for element in model.body:
         if hasattr(element, 'geometry') and len(element.geometry.vertices) >= 3:
             for a, b, c in pairs(element.geometry.vertices, count = 3):
                 if cross_product(b - a, c - a).length() < 1e-5:
-                    yield error(element, 'collinearity-error')
+                    yield report_problem('collinear', bad_object = element)
 
-def hairline_score(smallest_angle):
-    from math import log10
-    return max(0, -log10(smallest_angle))
-
+@problem_type('hairline-polygon',
+    severity = 'notice',
+    message = lambda smallest_angle: str.format(
+        'hairline polygon (smallest angle {})',
+        degree_rep(smallest_angle),
+    ),
+)
 def hairline_test(model):
     for element in model.body:
         if hasattr(element, 'geometry') and len(element.geometry.vertices) >= 3:
             smallest_angle = element.geometry.smallest_angle
             if smallest_angle < radians(0.5):
-                yield notice(element, 'hairline-warning',
+                yield report_problem(
+                    'hairline-polygon',
+                    bad_object = element,
                     smallest_angle = smallest_angle,
                 )
 
 manifest = {
-    'tests': {
-        'skew': skew_test,
-        'concave': concave_test,
-        'bowtie': bowtie_test,
-        'collinearity': collinear_test,
-        'hairline': hairline_test,
-    },
-    'messages': {
-        'skew-error': lambda skew_angle:
-            str.format('skew quadrilateral (plane angle {})',
-                degree_rep(skew_angle),
-            ),
-        'skew-warning': lambda skew_angle:
-            str.format('slightly skew quadrilateral (plane angle {})',
-                degree_rep(skew_angle),
-            ),
-        'concave-error': 'concave quadrilateral',
-        'self-intersecting': 'bowtie quadrilateral',
-        'collinearity-error': 'collinear polygon',
-        'hairline-warning': lambda smallest_angle:
-            str.format('hairline polygon (smallest angle {})',
-                degree_rep(smallest_angle),
-            ),
-    },
+    'tests': [
+        skew_test,
+        concave_test,
+        bowtie_test,
+        collinear_test,
+        hairline_test,
+    ],
 }
--- a/tests/subfiles.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/tests/subfiles.py	Sat Jun 08 01:32:25 2019 +0300
@@ -1,4 +1,4 @@
-from testsuite import warning, error
+from testsuite import problem_type, report_problem
 import testsuite
 from geometry import *
 from os.path import dirname
@@ -11,13 +11,17 @@
 with ini_path.open() as file:
     library_standards.read_file(file)
 
+@problem_type('zero-determinant',
+    severity = 'error',
+    message = 'matrix row or column all zero'
+)
 def determinant_test(model):
     '''
         Checks all subfile references for matrices with rows or columns all
         zero.
     '''
     yield from (
-        error(subfile_reference, 'zero-determinant')
+        report_problem('zero-determinant', bad_object = subfile_reference)
         for subfile_reference in model.subfile_references
         if abs(subfile_reference.matrix.determinant() - 0) < 1e-15
     )
@@ -28,10 +32,13 @@
         controls what axes are printed and can be used to filter away
         uninteresting values.
     '''
-    return ', '.join(
-        str.format('{} = {}', letter, getattr(scaling, letter))
-        for letter in axes
-    )
+    if isinstance(scaling, str):
+        return scaling
+    else:
+        return ', '.join(
+            str.format('{} = {}', letter, getattr(scaling, letter))
+            for letter in axes
+        )
 
 def check_scaling(scaling, axes):
     ''' Returns whether all given axes on the given scaling vector are 1. '''
@@ -51,6 +58,14 @@
     ]),
 }
 
+@problem_type('illegal-scaling',
+    severity = 'error',
+    message = lambda primitive, scaling, axes:
+        str.format('scaling of unscalable primitive {} ({})',
+            primitive,
+            scaling_description(scaling, axes),
+        ),
+)
 def scaling_legality_test(model):
     '''
         Checks the part against primitive references with bad scaling. Some
@@ -80,11 +95,43 @@
                 for axis in 'xyz'
                 if abs(getattr(scaling, axis) - 1) > 1e-5
             )
-            yield warning(subfile_reference, 'illegal-scaling',
+            yield report_problem('illegal-scaling',
+                bad_object = subfile_reference,
                 primitive = primitive,
                 axes = interesting_axes,
-                scaling = scaling)
+                scaling = scaling,
+            )
 
+@problem_type('cyclical-reference',
+    severity = 'error',
+    message = lambda chain:
+        str.format('cyclical subfile dependency: {chain}',
+            **locals(),
+        ),
+)
+@problem_type('bad-subfile',
+    severity = 'error',
+    message = lambda path, problem_text:
+        str.format('cannot process subfile "{path}": {problem_text}',
+            **locals(),
+        ),
+)
+@problem_type('moved-file-used',
+    severity = 'error',
+    message = lambda moved_file, new_file:
+        str.format('subfile "{moved_file}" has been moved to "{new_file}"',
+            **locals(),
+        ),
+)
+@problem_type('unnecessary-scaling',
+    severity = 'notice',
+    message = lambda scaled_flat_dimensions, scaling_vector:
+        str.format(
+            'subfile unnecessarily scaled in the {dims} ({scaling})',
+            dims = dimensions_description(scaled_flat_dimensions),
+            scaling = scaling_description(scaling_vector),
+        ),
+)
 def dependent_subfile_tests(model):
     '''
         Tests subfile references for such qualities that are dependent on the
@@ -104,11 +151,15 @@
                 subfile = cache.prepare_file(path)
             except filecache.CyclicalReferenceError as e:
                 failed_subfiles.add(path)
-                yield error(subfile_reference, 'cyclical-reference',
+                yield report_problem(
+                    'cyclical-reference',
+                    bad_object = subfile_reference,
                     chain = str(e),
                 )
             if not subfile.valid:
-                yield error(subfile_reference, 'bad-subfile',
+                yield report_problem(
+                    'bad-subfile',
+                    bad_object = subfile_reference,
                     path = path,
                     problem_text = subfile.problem,
                 )
@@ -117,9 +168,12 @@
                 import re
                 match = re.search(r'^\~Moved(?: to (\w+))?$', subfile.description)
                 if match:
-                    yield error(subfile_reference, 'moved-file-used',
+                    yield report_problem(
+                        'moved-file-used',
+                        bad_object = subfile_reference,
                         moved_file = path,
-                        new_file = match.group(1))
+                        new_file = match.group(1),
+                    )
                 scaling_vector = subfile_reference.matrix.scaling_vector()
                 scaled_dimensions = {
                     dimension
@@ -132,52 +186,30 @@
                 }
                 scaled_flat_dimensions = subfile.flatness & scaled_dimensions
                 if scaled_flat_dimensions:
-                    yield testsuite.notice(subfile_reference, 'unnecessary-scaling',
+                    yield report_problem(
+                        'unnecessary-scaling',
+                        bad_object = subfile_reference,
                         scaled_flat_dimensions = scaled_flat_dimensions,
                         scaling_vector = scaling_vector,
                     )
 
 def dimensions_description(dimensions):
-    sorted_dims = sorted(dimensions)
-    if len(sorted_dims) == 1:
-        return sorted_dims[0] + ' dimension'
+    if isinstance(dimensions, str):
+        return dimensions
     else:
-        return str.format('{} and {} dimensions',
-            ', '.join(sorted_dims[:-1]),
-            sorted_dims[-1],
-        )
+        sorted_dims = sorted(dimensions)
+        if len(sorted_dims) == 1:
+            return sorted_dims[0] + ' dimension'
+        else:
+            return str.format('{} and {} dimensions',
+                ', '.join(sorted_dims[:-1]),
+                sorted_dims[-1],
+            )
 
 manifest = {
-    'tests': {
-        'determinant': determinant_test,
-        'scaling-legality': scaling_legality_test,
-        'dependent-subfiles': dependent_subfile_tests,
-    },
-    'messages': {
-        'zero-determinant': 'matrix determinant is zero '
-            '(row or column all zero)',
-        'illegal-scaling': lambda primitive, scaling, axes:
-            str.format('scaling of unscalable primitive {} ({})',
-                primitive,
-                scaling_description(scaling, axes),
-            ),
-        'cyclical-reference': lambda chain:
-            str.format('cyclical subfile dependency: {chain}',
-                **locals(),
-            ),
-        'bad-subfile': lambda path, problem_text:
-            str.format('cannot process subfile "{path}": {problem_text}',
-                **locals(),
-            ),
-        'moved-file-used': lambda moved_file, new_file:
-            str.format('subfile "{moved_file}" has been moved to "{new_file}"',
-                **locals(),
-            ),
-        'unnecessary-scaling': lambda scaled_flat_dimensions, scaling_vector:
-            str.format(
-                'subfile unnecessarily scaled in the {dims} ({scaling})',
-                dims = dimensions_description(scaled_flat_dimensions),
-                scaling = scaling_description(scaling_vector),
-            )
-    },
+    'tests': [
+        determinant_test,
+        scaling_legality_test,
+        dependent_subfile_tests,
+    ],
 }
--- a/testsuite.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/testsuite.py	Sat Jun 08 01:32:25 2019 +0300
@@ -1,21 +1,46 @@
 from warnings import warn
 
-def report_element(bad_object, type, error_name, args):
-    return {
-        'type': type,
-        'object': bad_object,
-        'name': error_name,
-        'args': args,
-    }
+class ProblemType:
+    severities = ['error', 'notice'] # in descending order
+    def __init__(self, name, severity, message):
+        if severity not in ProblemType.severities:
+            raise ValueError(str.format(
+                'bad severity {severity!r}',
+                severity = severity,
+           ))
+        self.name = name
+        self.severity = severity
+        self.message = message
+    def __call__(self, bad_object, **args):
+        return Problem(
+            problem_class = self,
+            bad_object = bad_object,
+            **args,
+        )
 
-def warning(bad_object, error_name, **args):
-    return report_element(bad_object, 'warning', error_name, args)
+class Problem:
+    def __init__(self, problem_class, bad_object, **args):
+        self.problem_class = problem_class
+        self.severity = problem_class.severity
+        self.object = bad_object
+        self.args = args
+    def __str__(self):
+        if callable(self.problem_class.message):
+            return self.problem_class.message(**self.args)
+        else:
+            return self.problem_class.message
 
-def error(bad_object, error_name, **args):
-    return report_element(bad_object, 'error', error_name, args)
+def problem_type(problem_name, **args):
+    def wrapper(function):
+        if not hasattr(function, 'ldcheck_problem_types'):
+            function.ldcheck_problem_types = {}
+        new_type = ProblemType(name = problem_name, **args)
+        function.ldcheck_problem_types[problem_name] = new_type
+        return function
+    return wrapper
 
-def notice(bad_object, error_name, **args):
-    return report_element(bad_object, 'notice', error_name, args)
+def report_problem(problem_name, *, bad_object, **args):
+    return {'type': problem_name, 'bad-object': bad_object, 'args': args}
 
 def name_of_package(package):
     if isinstance(package, tuple):
@@ -34,51 +59,34 @@
         for result in walk_packages(tests.__path__)
     )
 
-def do_manifest_integrity_checks(test_suite, module):
-    '''
-        Runs integrity checks on a given module's manifest.
-    '''
-    def check_for_extra_keys():
-        extra_keys = module.manifest.keys() - test_suite.keys()
-        if extra_keys:
-            warn(str.format(
-                '{}: extra keys in manifest: {}',
-                module.__name__,
-                ', '.join(map(str, extra_keys))
-            ))
-    def check_for_manifest_duplicates():
-        for key in test_suite.keys():
-            duplicates = module.manifest[key].keys() & test_suite[key].keys()
-            if duplicates:
-                warn(str.format(
-                    '{}: redefined {} in manifests: {}',
-                    module.__name__,
-                    key,
-                    duplicates,
-                ))
-    check_for_extra_keys()
-    check_for_manifest_duplicates()
-
 def load_tests():
     '''
         Imports test modules and combines their manifests into a test suite.
     '''
-    test_suite = {'tests': {}, 'messages': {}}
+    test_suite = {'tests': []}
     for module_name in test_discovery():
         from importlib import import_module
         module = import_module(module_name)
         if hasattr(module, 'manifest'):
-            do_manifest_integrity_checks(test_suite, module)
             # Merge the data from the manifest
-            for key in module.manifest.keys() & test_suite.keys():
-                test_suite[key].update(module.manifest[key])
+            test_suite['tests'] += module.manifest['tests']
         else:
             warn(str.format('Module {} does not have a manifest', module_name))
+    test_suite['tests'].sort(key = lambda f: f.__name__)
     return test_suite
 
 def problem_key(problem):
-    problem_hierarchy = ['error', 'warning', 'notice']
-    return (problem_hierarchy.index(problem['type']), problem['line-number'])
+    rank = ProblemType.severities.index(problem.severity) # sort by severity
+    return (rank, problem.line_number)
+
+def build_problem(test_function, problem_params):
+    problem_name = problem_params['type']
+    problem_type = test_function.ldcheck_problem_types[problem_name]
+    problem_object = problem_type(
+        bad_object = problem_params['bad-object'],
+        **problem_params['args'],
+    )
+    return problem_object
 
 def check_model(model, test_suite = None):
     if not test_suite:
@@ -88,36 +96,38 @@
         element: (i, i + 1)
         for i, element in enumerate(model.body)
     }
-    for test_name, test_function in test_suite['tests'].items():
-        for problem in test_function(model):
-            problem['body-index'], problem['line-number'] \
-                = line_numbers[problem['object']]
-            del problem['object']
+    for test_function in test_suite['tests']:
+        for problem_params in test_function(model):
+            problem = build_problem(test_function, problem_params)
+            # add line numbers to the problem
+            problem.body_index, problem.line_number \
+                = line_numbers[problem.object]
+            problem.object = None
             problems.append(problem)
     return {
         'passed': not any(
-            problem['type'] == 'error'
+            problem.severity == 'error'
             for problem in problems
         ),
         'problems': sorted(problems, key = problem_key),
     }
 
 def problem_text(problem, test_suite):
-    message = test_suite['messages'][problem['name']]
+    message = problem.problem_class.message
     if callable(message):
-        message = message(**problem['args'])
+        message = message(**problem.args)
     return message
 
 def format_report_html(report, model, test_suite):
     messages = []
     for problem in report['problems']:
-        ldraw_code = model.body[problem['body-index']].textual_representation()
+        ldraw_code = model.body[problem.body_index].textual_representation()
         message = str.format(
             '<li class="{problem_type}">{model_name}:{line_number}:'
             '{problem_type}: {message}<br />{ldraw_code}</li>',
             model_name = model.name,
-            line_number = problem['line-number'],
-            problem_type = problem['type'],
+            line_number = problem.line_number,
+            problem_type = problem.severity,
             message = problem_text(problem, test_suite),
             ldraw_code = ldraw_code,
         )
@@ -129,22 +139,20 @@
     colorama.init()
     messages = []
     for problem in report['problems']:
-        if problem['type'] == 'error':
+        if problem.severity == 'error':
             text_colour = colorama.Fore.LIGHTRED_EX
-        elif problem['type'] == 'warning':
-            text_colour = colorama.Fore.LIGHTYELLOW_EX
-        elif problem['type'] == 'notice':
+        elif problem.severity == 'notice':
             text_colour = colorama.Fore.LIGHTBLUE_EX
         else:
             text_colour = ''
-        ldraw_code = model.body[problem['body-index']].textual_representation()
+        ldraw_code = model.body[problem.body_index].textual_representation()
         message = str.format(
             '{text_colour}{model_name}:{line_number}: {problem_type}: {message}'
             '{colour_reset}\n\t{ldraw_code}',
             text_colour = text_colour,
             model_name = model.name,
-            line_number = problem['line-number'],
-            problem_type = problem['type'],
+            line_number = problem.line_number,
+            problem_type = problem.severity,
             message = problem_text(problem, test_suite),
             colour_reset = colorama.Fore.RESET,
             ldraw_code = ldraw_code,
@@ -152,6 +160,10 @@
         messages.append(message)
     return '\n'.join(messages)
 
+def all_warning_types(test_suite):
+    for test_function in test_suite['tests']:
+        yield from test_function.ldcheck_problem_types.values()
+
 if __name__ == '__main__':
     from pprint import pprint
     pprint(load_tests())
--- a/webfront.py	Wed Jun 05 00:33:50 2019 +0300
+++ b/webfront.py	Sat Jun 08 01:32:25 2019 +0300
@@ -28,9 +28,9 @@
 
         # Amend human-readable messages into the report
         for problem in report['problems']:
-            object = model.body[problem['body-index']]
-            problem['message'] = problem_text(problem, test_suite)
-            problem['ldraw-code'] = object.textual_representation()
+            object = model.body[problem.body_index]
+            problem.message_str = problem_text(problem, test_suite)
+            problem.ldraw_code = object.textual_representation()
     else:
         report = None
         filename = None

mercurial