From 0c7807e0cdbf2bc9b8463822079217bc75b3520d Mon Sep 17 00:00:00 2001 From: jspijker Date: Sun, 20 Nov 2022 12:16:37 +0100 Subject: [PATCH 1/5] Create a proper Pyhton packages for printer-linter Such that it can be downloaded, installed and used locally and on other repo's actions etc --- printer-linter/pyproject.toml | 17 ++++++++++ printer-linter/setup.cfg | 10 ++++++ printer-linter/setup.py | 6 ++++ .../{tidy => src/printerlinter}/__init__.py | 0 .../{tidy => src/printerlinter}/defintion.py | 0 .../{tidy => src/printerlinter}/diagnostic.py | 0 .../{tidy => src/printerlinter}/meshes.py | 0 .../{tidy => src/printerlinter}/profile.py | 0 .../printerlinter/terminal.py} | 33 +++++++++++-------- 9 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 printer-linter/pyproject.toml create mode 100644 printer-linter/setup.cfg create mode 100644 printer-linter/setup.py rename printer-linter/{tidy => src/printerlinter}/__init__.py (100%) rename printer-linter/{tidy => src/printerlinter}/defintion.py (100%) rename printer-linter/{tidy => src/printerlinter}/diagnostic.py (100%) rename printer-linter/{tidy => src/printerlinter}/meshes.py (100%) rename printer-linter/{tidy => src/printerlinter}/profile.py (100%) rename printer-linter/{printer-linter.py => src/printerlinter/terminal.py} (95%) diff --git a/printer-linter/pyproject.toml b/printer-linter/pyproject.toml new file mode 100644 index 0000000000..bf04261c7d --- /dev/null +++ b/printer-linter/pyproject.toml @@ -0,0 +1,17 @@ +[project] +name = "printerlinter" +description = "Cura UltiMaker printer linting tool" +version = "0.1.0" +authors = [ + { name = "UltiMaker", email = "cura@ultimaker.com" } +] +dependencies = [ + "pyyaml" +] + +[project.scripts] +printer-linter = "printerlinter.terminal:main" + +[build-system] +requires = ["setuptools"] +build-backend = "setuptools.build_meta" \ No newline at end of file diff --git a/printer-linter/setup.cfg b/printer-linter/setup.cfg new file mode 100644 index 0000000000..68b0484162 --- /dev/null +++ b/printer-linter/setup.cfg @@ -0,0 +1,10 @@ +[metadata] +name = printerlinter + +[options] +package_dir= + =src +packages=find: + +[options.packages.find] +where=src \ No newline at end of file diff --git a/printer-linter/setup.py b/printer-linter/setup.py new file mode 100644 index 0000000000..25536050b2 --- /dev/null +++ b/printer-linter/setup.py @@ -0,0 +1,6 @@ +#!/usr/bin/env python + +from setuptools import setup + +if __name__ == "__main__": + setup() diff --git a/printer-linter/tidy/__init__.py b/printer-linter/src/printerlinter/__init__.py similarity index 100% rename from printer-linter/tidy/__init__.py rename to printer-linter/src/printerlinter/__init__.py diff --git a/printer-linter/tidy/defintion.py b/printer-linter/src/printerlinter/defintion.py similarity index 100% rename from printer-linter/tidy/defintion.py rename to printer-linter/src/printerlinter/defintion.py diff --git a/printer-linter/tidy/diagnostic.py b/printer-linter/src/printerlinter/diagnostic.py similarity index 100% rename from printer-linter/tidy/diagnostic.py rename to printer-linter/src/printerlinter/diagnostic.py diff --git a/printer-linter/tidy/meshes.py b/printer-linter/src/printerlinter/meshes.py similarity index 100% rename from printer-linter/tidy/meshes.py rename to printer-linter/src/printerlinter/meshes.py diff --git a/printer-linter/tidy/profile.py b/printer-linter/src/printerlinter/profile.py similarity index 100% rename from printer-linter/tidy/profile.py rename to printer-linter/src/printerlinter/profile.py diff --git a/printer-linter/printer-linter.py b/printer-linter/src/printerlinter/terminal.py similarity index 95% rename from printer-linter/printer-linter.py rename to printer-linter/src/printerlinter/terminal.py index ba1387c5de..565818ca4e 100644 --- a/printer-linter/printer-linter.py +++ b/printer-linter/src/printerlinter/terminal.py @@ -8,7 +8,7 @@ from pathlib import Path import yaml -from tidy import create +from . import create def examineFile(file, settings): @@ -72,7 +72,24 @@ def formatFile(file: Path, settings): config.write(f, space_around_delimiters=settings["format"].get("format-profile-space-around-delimiters", True)) -def main(files, setting_path, to_format, to_fix, to_diagnose, report): +def 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("--diagnose", action="store_true", help="Diagnose 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() + files = args.Files + setting_path = args.setting + to_format = args.format + to_fix = args.fix + to_diagnose = args.diagnose + report = args.report + if not setting_path: setting_path = Path(getcwd(), ".printer-linter") @@ -118,14 +135,4 @@ def main(files, setting_path, to_format, to_fix, to_diagnose, report): 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("--diagnose", action="store_true", help="Diagnose 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.diagnose, args.report) + main() From 2b9080732664168b152868b91c70c0b036ef954b Mon Sep 17 00:00:00 2001 From: jspijker Date: Sun, 20 Nov 2022 12:38:22 +0100 Subject: [PATCH 2/5] Update gitignore with python build files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 26fe1ccf4a..1e8fd47664 100644 --- a/.gitignore +++ b/.gitignore @@ -99,3 +99,4 @@ conanbuildinfo.txt graph_info.json Ultimaker-Cura.spec .run/ +/printer-linter/src/printerlinter.egg-info/ From 4b455d45e9ec66e5f24592bfd71f74b76b09d2e6 Mon Sep 17 00:00:00 2001 From: jspijker Date: Mon, 21 Nov 2022 10:51:08 +0100 Subject: [PATCH 3/5] Use same yaml structure as cling-tidy --- printer-linter/pyproject.toml | 2 +- printer-linter/src/printerlinter/__init__.py | 15 +-- printer-linter/src/printerlinter/defintion.py | 31 +++--- .../src/printerlinter/diagnostic.py | 95 +++---------------- printer-linter/src/printerlinter/factory.py | 17 ++++ printer-linter/src/printerlinter/meshes.py | 19 ++-- .../src/printerlinter/replacement.py | 12 +++ .../src/{printerlinter => }/terminal.py | 18 ++-- 8 files changed, 85 insertions(+), 124 deletions(-) create mode 100644 printer-linter/src/printerlinter/factory.py create mode 100644 printer-linter/src/printerlinter/replacement.py rename printer-linter/src/{printerlinter => }/terminal.py (90%) diff --git a/printer-linter/pyproject.toml b/printer-linter/pyproject.toml index bf04261c7d..74c6531c87 100644 --- a/printer-linter/pyproject.toml +++ b/printer-linter/pyproject.toml @@ -10,7 +10,7 @@ dependencies = [ ] [project.scripts] -printer-linter = "printerlinter.terminal:main" +printer-linter = "terminal:main" [build-system] requires = ["setuptools"] diff --git a/printer-linter/src/printerlinter/__init__.py b/printer-linter/src/printerlinter/__init__.py index 9bf761bd5e..8023686891 100644 --- a/printer-linter/src/printerlinter/__init__.py +++ b/printer-linter/src/printerlinter/__init__.py @@ -1,20 +1,7 @@ from .defintion import Definition from .diagnostic import Diagnostic +from .factory import create 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/src/printerlinter/defintion.py b/printer-linter/src/printerlinter/defintion.py index 984bdd13e5..a2fb3a4ff0 100644 --- a/printer-linter/src/printerlinter/defintion.py +++ b/printer-linter/src/printerlinter/defintion.py @@ -1,7 +1,9 @@ import json +import re from pathlib import Path from .diagnostic import Diagnostic +from .replacement import Replacement class Definition: @@ -11,6 +13,8 @@ class Definition: self._defs = {} self._getDefs(file) + self._content = self._file.read_text() + settings = {} for k, v in self._defs["fdmprinter"]["settings"].items(): self._getSetting(k, v, settings) @@ -32,24 +36,25 @@ class Definition: 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) + redefined = re.compile(r'.*(\"' + key + r'\"[\s\S]*?\{)[\s\S]*?(\}[,\"]?)') + found = redefined.search(self._content) + yield Diagnostic( + file = self._file, + diagnostic_name = "diagnostic-definition-redundant-override", + message = f"Overriding **{key}** with the same value (**{value}**) as defined in parent definition: **{definition['inherits']}**", + level = "Warning", + offset = found.span(0)[0], + replacements = [Replacement( + file = self._file, + offset = found.span(1)[0], + length = found.span(2)[1] - found.span(1)[0], + replacement_text = "")] + ) def checkValueOutOfBounds(self): - pass def _getSetting(self, name, setting, settings): diff --git a/printer-linter/src/printerlinter/diagnostic.py b/printer-linter/src/printerlinter/diagnostic.py index e828751695..7941929c43 100644 --- a/printer-linter/src/printerlinter/diagnostic.py +++ b/printer-linter/src/printerlinter/diagnostic.py @@ -1,85 +1,20 @@ class Diagnostic: - def __init__(self, illness, msg, file, key=None, termination_seq=None): - self.illness = illness - self.key = key - self.msg = msg + def __init__(self, file, diagnostic_name, message, level, offset, replacements=None): 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"] - 1 - end_line = self.location["end"]["line"] - 1 - self._content_block = "\n".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"] - 2 - start_col = 0 - end_line = self.location["end"]["line"] - 1 - end_col = len(self._lines[start_line:end_line - 1]) + self.location["start"]["col"] - 4 # TODO: double check if 4 holds in all instances - self._fix = self.content_block[start_col:end_col] - return self._fix + self.diagnostic_name = diagnostic_name + self.message = message + self.offset = offset + self.level = level + self.replacements = replacements 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} + diagnostic_dict = {"DiagnosticName": self.diagnostic_name, + "DiagnosticMessage": { + "Message": self.message, + "FilePath": self.file.as_posix(), + "FileOffset": self.offset, + "Replacements": [] if self.replacements is None else [r.toDict() for r in self.replacements], + }, + "Level": self.level + } return diagnostic_dict diff --git a/printer-linter/src/printerlinter/factory.py b/printer-linter/src/printerlinter/factory.py new file mode 100644 index 0000000000..12e2d36628 --- /dev/null +++ b/printer-linter/src/printerlinter/factory.py @@ -0,0 +1,17 @@ +from .profile import Profile +from .defintion import Definition +from .meshes import Meshes + + +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 \ No newline at end of file diff --git a/printer-linter/src/printerlinter/meshes.py b/printer-linter/src/printerlinter/meshes.py index ae0eb5ab02..2e2c5784c0 100644 --- a/printer-linter/src/printerlinter/meshes.py +++ b/printer-linter/src/printerlinter/meshes.py @@ -20,15 +20,22 @@ class Meshes: 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 Diagnostic( + file = self._file, + diagnostic_name = "diagnostic-mesh-file-extension", + message = f"Extension **{self._file.suffix}** not supported, use **3mf**, **obj** or **stl**", + level = "Error" + ) 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 Diagnostic( + file = self._file, + diagnostic_name = "diagnostic-mesh-file-size", + message = f"Mesh file with a size **{self._file.stat().st_size}** is bigger then allowed maximum of **{self._max_file_size}**", + level = "Error", + offset = 0 + ) yield diff --git a/printer-linter/src/printerlinter/replacement.py b/printer-linter/src/printerlinter/replacement.py new file mode 100644 index 0000000000..d609071875 --- /dev/null +++ b/printer-linter/src/printerlinter/replacement.py @@ -0,0 +1,12 @@ +class Replacement: + def __init__(self, file, offset, length, replacement_text): + self.file = file + self.offset = offset + self.length = length + self.replacement_text = replacement_text + + def toDict(self): + return {"FilePath": self.file.as_posix(), + "Offset": self.offset, + "Length": self.length, + "ReplacementText": self.replacement_text} diff --git a/printer-linter/src/printerlinter/terminal.py b/printer-linter/src/terminal.py similarity index 90% rename from printer-linter/src/printerlinter/terminal.py rename to printer-linter/src/terminal.py index 565818ca4e..a21ee819df 100644 --- a/printer-linter/src/printerlinter/terminal.py +++ b/printer-linter/src/terminal.py @@ -8,22 +8,20 @@ from pathlib import Path import yaml -from . import create +from printerlinter import factory def examineFile(file, settings): - patient = create(file, settings) + patient = factory.create(file, settings) if patient is None: return {} - full_body_check = {f"{file.as_posix()}": []} + body_check = [] for diagnostic in patient.check(): if diagnostic: - full_body_check[f"{file.as_posix()}"].append(diagnostic.toDict()) + body_check.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 + return body_check def fixFile(file, settings, full_body_check): @@ -101,13 +99,13 @@ def main(): settings = yaml.load(f, yaml.FullLoader) if to_fix or to_diagnose: - full_body_check = {} + full_body_check = {"Diagnostics": []} for file in files: if file.is_dir(): for fp in file.rglob("**/*"): - full_body_check |= examineFile(fp, settings) + full_body_check["Diagnostics"].append(examineFile(fp, settings)) else: - full_body_check |= examineFile(file, settings) + full_body_check["Diagnostics"].append(examineFile(file, settings)) results = yaml.dump(full_body_check, default_flow_style=False, indent=4, width=240) if report: From 0070cb7af09c29e04e6209436fd00e560dd73f94 Mon Sep 17 00:00:00 2001 From: jspijker Date: Mon, 21 Nov 2022 10:58:28 +0100 Subject: [PATCH 4/5] flatten list --- printer-linter/src/terminal.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index a21ee819df..55af41281f 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -11,17 +11,14 @@ import yaml from printerlinter import factory -def examineFile(file, settings): +def examineFile(file, settings, full_body_check): patient = factory.create(file, settings) if patient is None: - return {} + return - body_check = [] for diagnostic in patient.check(): if diagnostic: - body_check.append(diagnostic.toDict()) - - return body_check + full_body_check["Diagnostics"].append(diagnostic.toDict()) def fixFile(file, settings, full_body_check): @@ -103,9 +100,9 @@ def main(): for file in files: if file.is_dir(): for fp in file.rglob("**/*"): - full_body_check["Diagnostics"].append(examineFile(fp, settings)) + examineFile(fp, settings, full_body_check) else: - full_body_check["Diagnostics"].append(examineFile(file, settings)) + examineFile(file, settings, full_body_check) results = yaml.dump(full_body_check, default_flow_style=False, indent=4, width=240) if report: From 67688aad95f32fa33f5ee53a4896ab48e1bd42a3 Mon Sep 17 00:00:00 2001 From: jspijker Date: Mon, 21 Nov 2022 13:11:52 +0100 Subject: [PATCH 5/5] Formatted the messages --- printer-linter/src/printerlinter/defintion.py | 2 +- printer-linter/src/printerlinter/meshes.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/printer-linter/src/printerlinter/defintion.py b/printer-linter/src/printerlinter/defintion.py index a2fb3a4ff0..a92828f31b 100644 --- a/printer-linter/src/printerlinter/defintion.py +++ b/printer-linter/src/printerlinter/defintion.py @@ -44,7 +44,7 @@ class Definition: yield Diagnostic( file = self._file, diagnostic_name = "diagnostic-definition-redundant-override", - message = f"Overriding **{key}** with the same value (**{value}**) as defined in parent definition: **{definition['inherits']}**", + message = f"Overriding {key} with the same value ({value}) as defined in parent definition: {definition['inherits']}", level = "Warning", offset = found.span(0)[0], replacements = [Replacement( diff --git a/printer-linter/src/printerlinter/meshes.py b/printer-linter/src/printerlinter/meshes.py index 2e2c5784c0..404f194100 100644 --- a/printer-linter/src/printerlinter/meshes.py +++ b/printer-linter/src/printerlinter/meshes.py @@ -23,19 +23,19 @@ class Meshes: yield Diagnostic( file = self._file, diagnostic_name = "diagnostic-mesh-file-extension", - message = f"Extension **{self._file.suffix}** not supported, use **3mf**, **obj** or **stl**", - level = "Error" + message = f"Extension {self._file.suffix} not supported, use 3mf, obj or stl", + level = "Error", + offset = 1 ) yield def checkFileSize(self): - if self._file.stat().st_size > self._max_file_size: yield Diagnostic( file = self._file, diagnostic_name = "diagnostic-mesh-file-size", - message = f"Mesh file with a size **{self._file.stat().st_size}** is bigger then allowed maximum of **{self._max_file_size}**", + message = f"Mesh file with a size {self._file.stat().st_size} is bigger then allowed maximum of {self._max_file_size}", level = "Error", - offset = 0 + offset = 1 ) yield