From 104bc585f44fb7665454443b46936c4024fee29e Mon Sep 17 00:00:00 2001 From: jspijker Date: Sat, 19 Nov 2022 19:07:32 +0100 Subject: [PATCH] Added a linting tool for Cura Printers and Profiles printer-linter works of definitions, profiles and meshes; It has various diagnostic checks. With possible suggestions for fixes. It should also be able to fix certain diagnostic issues and it can be used to format the files according to code-style. It can output the diagnostics in a yaml file, which can then be used to comment on PR's with suggestions to the author. Future PR. The settings for the diagnostics and checks are defined in `.printer-linter` and are very self explanatory. ``` checks: diagnostic-mesh-file-extension: true diagnostic-mesh-file-size: true diagnostic-definition-redundant-override: true fixes: diagnostic-definition-redundant-override: true format: format-definition-bracket-newline: false format-definition-paired-coordinate-array: true format-definition-sort-keys: true format-definition-indent: 4 format-profile-space-around-delimiters: true format-profile-sort-keys: true diagnostic-mesh-file-size: 1200000 ``` --- .printer-linter | 14 ++++ printer-linter/printer-linter.py | 129 ++++++++++++++++++++++++++++++ printer-linter/tidy/__init__.py | 20 +++++ printer-linter/tidy/defintion.py | 96 ++++++++++++++++++++++ printer-linter/tidy/diagnostic.py | 87 ++++++++++++++++++++ printer-linter/tidy/meshes.py | 34 ++++++++ printer-linter/tidy/profile.py | 7 ++ requirements-dev.txt | 1 + 8 files changed, 388 insertions(+) create mode 100644 .printer-linter create mode 100644 printer-linter/printer-linter.py create mode 100644 printer-linter/tidy/__init__.py create mode 100644 printer-linter/tidy/defintion.py create mode 100644 printer-linter/tidy/diagnostic.py create mode 100644 printer-linter/tidy/meshes.py create mode 100644 printer-linter/tidy/profile.py diff --git a/.printer-linter b/.printer-linter new file mode 100644 index 0000000000..085937325e --- /dev/null +++ b/.printer-linter @@ -0,0 +1,14 @@ +checks: + diagnostic-mesh-file-extension: true + diagnostic-mesh-file-size: true + diagnostic-definition-redundant-override: true +fixes: + diagnostic-definition-redundant-override: true +format: + format-definition-bracket-newline: false + format-definition-paired-coordinate-array: true + format-definition-sort-keys: true + format-definition-indent: 4 + format-profile-space-around-delimiters: true + format-profile-sort-keys: true +diagnostic-mesh-file-size: 800000 \ No newline at end of file diff --git a/printer-linter/printer-linter.py b/printer-linter/printer-linter.py new file mode 100644 index 0000000000..7dd18e6176 --- /dev/null +++ b/printer-linter/printer-linter.py @@ -0,0 +1,129 @@ +import configparser +import json +import re +from argparse import ArgumentParser +from collections import OrderedDict +from os import getcwd +from pathlib import Path + +import yaml + +from tidy import create + + +def examineFile(file, settings): + patient = create(file, settings) + if patient is None: + return {} + + full_body_check = {f"{file.as_posix()}": []} + for diagnostic in patient.check(): + if diagnostic: + full_body_check[f"{file.as_posix()}"].append(diagnostic.toDict()) + + if len(full_body_check[f"{file.as_posix()}"]) == 0: + del full_body_check[f"{file.as_posix()}"] + return full_body_check + + +def fixFile(file, settings, full_body_check): + if not file.exists(): + return + ext = ".".join(file.name.split(".")[-2:]) + + if ext == "def.json": + issues = full_body_check[f"{file.as_posix()}"] + for issue in issues: + if issue["diagnostic"] == "diagnostic-definition-redundant-override" and settings["fixes"].get( + "diagnostic-definition-redundant-override", True): + pass + + +def formatFile(file: Path, settings): + if not file.exists(): + return + ext = ".".join(file.name.split(".")[-2:]) + + if ext == "def.json": + definition = json.loads(file.read_text()) + content = json.dumps(definition, indent=settings["format"].get("format-definition-indent", 4), + sort_keys=settings["format"].get("format-definition-sort-keys", True)) + + if settings["format"].get("format-definition-bracket-newline", True): + newline = re.compile(r"(\B\s+)(\"[\w\"]+)(\:\s\{)") + content = newline.sub(r"\1\2:\1{", content) + + if settings["format"].get("format-definition-paired-coordinate-array", True): + paired_coordinates = re.compile(r"(\[)\s+(-?\d*),\s*(-?\d*)\s*(\])") + content = paired_coordinates.sub(r"\1 \2, \3 \4", content) + + file.write_text(content) + + if ext == "inst.cfg": + config = configparser.ConfigParser() + config.read(file) + + if settings["format"].get("format-profile-sort-keys", True): + for section in config._sections: + config._sections[section] = OrderedDict(sorted(config._sections[section].items(), key=lambda t: t[0])) + config._sections = OrderedDict(sorted(config._sections.items(), key=lambda t: t[0])) + + with open(file, "w") as f: + config.write(f, space_around_delimiters=settings["format"].get("format-profile-space-around-delimiters", True)) + + +def main(files, setting_path, to_format, to_fix, report): + if not setting_path: + setting_path = Path(getcwd(), ".printer-linter") + + if not setting_path.exists(): + print(f"Can't find the settings: {setting_path}") + return + + with open(setting_path, "r") as f: + settings = yaml.load(f, yaml.FullLoader) + + full_body_check = {} + for file in files: + if file.is_dir(): + for fp in file.rglob("**/*"): + full_body_check |= examineFile(fp, settings) + else: + full_body_check |= examineFile(file, settings) + + results = yaml.dump(full_body_check, default_flow_style=False, indent=4, width=240) + if report: + report.write_text(results) + else: + print(results) + + if to_fix: + for file in files: + if file.is_dir(): + for fp in file.rglob("**/*"): + if f"{file.as_posix()}" in full_body_check: + fixFile(fp, settings, full_body_check) + else: + if f"{file.as_posix()}" in full_body_check: + fixFile(file, settings, full_body_check) + + if to_format: + for file in files: + if file.is_dir(): + for fp in file.rglob("**/*"): + formatFile(fp, settings) + else: + formatFile(file, settings) + + +if __name__ == "__main__": + parser = ArgumentParser( + description="UltiMaker Cura printer linting, static analysis and formatting of Cura printer definitions and other resources") + parser.add_argument("--setting", required=False, type=Path, help="Path to the `.printer-linter` setting file") + parser.add_argument("--report", required=False, type=Path, help="Path where the diagnostic report should be stored") + parser.add_argument("--format", action="store_true", help="Format the files") + parser.add_argument("--fix", action="store_true", help="Attempt to apply the suggested fixes on the files") + parser.add_argument("Files", metavar="F", type=Path, nargs="+", help="Files or directories to format") + + args = parser.parse_args() + main(args.Files, args.setting, args.format, args.fix, args.report) diff --git a/printer-linter/tidy/__init__.py b/printer-linter/tidy/__init__.py new file mode 100644 index 0000000000..9bf761bd5e --- /dev/null +++ b/printer-linter/tidy/__init__.py @@ -0,0 +1,20 @@ +from .defintion import Definition +from .diagnostic import Diagnostic +from .meshes import Meshes +from .profile import Profile + +__all__ = ["Profile", "Definition", "Meshes", "Diagnostic", "create"] + + +def create(file, settings): + if not file.exists(): + return None + if ".inst" in file.suffixes and ".cfg" in file.suffixes: + return Profile(file, settings) + if ".def" in file.suffixes and ".json" in file.suffixes: + if file.stem in ("fdmprinter.def", "fdmextruder.def"): + return None + return Definition(file, settings) + if file.parent.stem == "meshes": + return Meshes(file, settings) + return None diff --git a/printer-linter/tidy/defintion.py b/printer-linter/tidy/defintion.py new file mode 100644 index 0000000000..984bdd13e5 --- /dev/null +++ b/printer-linter/tidy/defintion.py @@ -0,0 +1,96 @@ +import json +from pathlib import Path + +from .diagnostic import Diagnostic + + +class Definition: + def __init__(self, file, settings): + self._settings = settings + self._file = file + self._defs = {} + self._getDefs(file) + + settings = {} + for k, v in self._defs["fdmprinter"]["settings"].items(): + self._getSetting(k, v, settings) + self._defs["fdmprinter"] = {"overrides": settings} + + def check(self): + if self._settings["checks"].get("diagnostic-definition-redundant-override", False): + for check in self.checkRedefineOverride(): + yield check + + # Add other which will yield Diagnostic's + # TODO: A check to determine if the user set value is with the min and max value defined in the parent and doesn't trigger a warning + # TODO: A check if the key exist in the first place + # TODO: Check if the model platform exist + + yield + + def checkRedefineOverride(self): + definition_name = list(self._defs.keys())[0] + definition = self._defs[definition_name] + if "overrides" in definition and definition_name != "fdmprinter": + keys = list(definition["overrides"].keys()) + for key, value_dict in definition["overrides"].items(): + is_redefined, value, parent = self._isDefinedInParent(key, value_dict, definition['inherits']) + if is_redefined: + termination_key = keys.index(key) + 1 + if termination_key >= len(keys): + # FIXME: find the correct end sequence for now assume it is on the same line + termination_seq = None + else: + termination_seq = keys[termination_key] + yield Diagnostic("diagnostic-definition-redundant-override", + f"Overriding **{key}** with the same value (**{value}**) as defined in parent definition: **{definition['inherits']}**", + self._file, + key, + termination_seq) + + def checkValueOutOfBounds(self): + + pass + + def _getSetting(self, name, setting, settings): + if "children" in setting: + for childname, child in setting["children"].items(): + self._getSetting(childname, child, settings) + settings |= {name: setting} + + def _getDefs(self, file): + if not file.exists(): + return + self._defs[Path(file.stem).stem] = json.loads(file.read_text()) + if "inherits" in self._defs[Path(file.stem).stem]: + parent_file = file.parent.joinpath(f"{self._defs[Path(file.stem).stem]['inherits']}.def.json") + self._getDefs(parent_file) + + def _isDefinedInParent(self, key, value_dict, inherits_from): + if "overrides" not in self._defs[inherits_from]: + return self._isDefinedInParent(key, value_dict, self._defs[inherits_from]["inherits"]) + + parent = self._defs[inherits_from]["overrides"] + is_number = self._defs["fdmprinter"]["overrides"][key] in ("float", "int") + for value in value_dict.values(): + if key in parent: + check_values = [cv for cv in [parent[key].get("default_value", None), parent[key].get("value", None)] if cv is not None] + for check_value in check_values: + if is_number: + try: + v = str(float(value)) + except: + v = value + try: + cv = str(float(check_value)) + except: + cv = check_value + else: + v = value + cv = check_value + if v == cv: + return True, value, parent + + if "inherits" in parent: + return self._isDefinedInParent(key, value_dict, parent["inherits"]) + return False, None, None diff --git a/printer-linter/tidy/diagnostic.py b/printer-linter/tidy/diagnostic.py new file mode 100644 index 0000000000..f3ef618d52 --- /dev/null +++ b/printer-linter/tidy/diagnostic.py @@ -0,0 +1,87 @@ +class Diagnostic: + def __init__(self, illness, msg, file, key=None, termination_seq=None): + self.illness = illness + self.key = key + self.msg = msg + self.file = file + self._lines = None + self._location = None + self._fix = None + self._content_block = None + self._termination_seq = termination_seq + + @property + def location(self): + if self._location: + return self._location + if not self._lines: + with open(self.file, "r") as f: + if not self.is_text_file: + self._fix = "" + return self._fix + self._lines = f.readlines() + + start_location = {"col": 1, "line": 1} + end_location = {"col": len(self._lines[-1]) + 1, "line": len(self._lines) + 1} + + if self.key is not None: + for lino, line in enumerate(self._lines, 1): + if f'"{self.key}":' in line: + col = line.index(f'"{self.key}":') + 1 + start_location = {"col": col, "line": lino} + if self._termination_seq is None: + end_location = {"col": len(line) + 1, "line": lino} + break + if f'"{self._termination_seq}":' in line: + col = line.index(f'"{self._termination_seq}":') + 1 + end_location = {"col": col, "line": lino} + self._location = {"start": start_location, "end": end_location} + return self._location + + @property + def is_text_file(self): + return self.file.name.split(".", maxsplit=1)[-1] in ("def.json", "inst.cfg") + + @property + def content_block(self): + if self._content_block: + return self._content_block + + if not self._lines: + if not self.is_text_file: + self._fix = "" + return self._fix + with open(self.file, "r") as f: + self._lines = f.readlines() + + start_line = self.location["start"]["line"] + start_col = self.location["start"]["col"] + end_line = self.location["end"]["line"] + end_col = len(self._lines[start_line:end_line - 1]) + self.location["start"]["col"] + self._content_block = "".join(self._lines[start_line:end_line]) + return self._content_block + + @property + def fix(self): + if self._fix: + return self._fix + + if not self._lines: + if not self.is_text_file: + self._fix = "" + return self._fix + with open(self.file, "r") as f: + self._lines = f.readlines() + + start_line = self.location["start"]["line"] + start_col = self.location["start"]["col"] + end_line = self.location["end"]["line"] + end_col = len(self._lines[start_line:end_line - 1]) + self.location["start"]["col"] + self._fix = self.content_block[start_col:end_col] + return self._fix + + def toDict(self): + diagnostic_dict = {"diagnostic": self.illness, "message": self.msg} + if self.is_text_file: + diagnostic_dict |= {"fix": self.fix, "lino": self.location, "content": self.content_block} + return diagnostic_dict diff --git a/printer-linter/tidy/meshes.py b/printer-linter/tidy/meshes.py new file mode 100644 index 0000000000..ae0eb5ab02 --- /dev/null +++ b/printer-linter/tidy/meshes.py @@ -0,0 +1,34 @@ +from .diagnostic import Diagnostic + + +class Meshes: + def __init__(self, file, settings): + self._settings = settings + self._file = file + self._max_file_size = self._settings.get("diagnostic-mesh-file-size", 1e6) + + def check(self): + if self._settings["checks"].get("diagnostic-mesh-file-extension", False): + for check in self.checkFileFormat(): + yield check + + if self._settings["checks"].get("diagnostic-mesh-file-size", False): + for check in self.checkFileSize(): + yield check + + yield + + def checkFileFormat(self): + if self._file.suffix.lower() not in (".3mf", ".obj", ".stl"): + yield Diagnostic("diagnostic-mesh-file-extension", + f"Extension **{self._file.suffix}** not supported, use **3mf**, **obj** or **stl**", + self._file) + yield + + def checkFileSize(self): + + if self._file.stat().st_size > self._max_file_size: + yield Diagnostic("diagnostic-mesh-file-size", + f"Mesh file with a size **{self._file.stat().st_size}** is bigger then allowed maximum of **{self._max_file_size}**", + self._file) + yield diff --git a/printer-linter/tidy/profile.py b/printer-linter/tidy/profile.py new file mode 100644 index 0000000000..137344fe71 --- /dev/null +++ b/printer-linter/tidy/profile.py @@ -0,0 +1,7 @@ +class Profile: + def __init__(self, file, settings): + self._settings = settings + self._file = file + + def check(self): + yield diff --git a/requirements-dev.txt b/requirements-dev.txt index 819943e8b7..b1e52571ca 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,5 +1,6 @@ pytest pyinstaller pyinstaller-hooks-contrib +pyyaml sip==6.5.1 jinja2