diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 57c3732d81..1be20c4548 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -18,6 +18,7 @@ jobs: - uses: technote-space/get-diff-action@v6 with: + DIFF_FILTER: AMRCD PATTERNS: | resources/+(extruders|definitions)/*.def.json resources/+(intent|quality|variants)/**/*.inst.cfg @@ -41,6 +42,10 @@ jobs: if: env.GIT_DIFF && !env.MATCHED_FILES run: python printer-linter/src/terminal.py --diagnose --report printer-linter-result/fixes.yml ${{ env.GIT_DIFF_FILTERED }} + - name: Check Deleted Files(s) + if: env.GIT_DIFF + run: python printer-linter/src/terminal.py --deleted --report printer-linter-result/comment.md ${{ env.GIT_DIFF_FILTERED }} + - name: Save PR metadata run: | echo ${{ github.event.number }} > printer-linter-result/pr-id.txt diff --git a/.github/workflows/printer-linter-pr-post.yml b/.github/workflows/printer-linter-pr-post.yml index 81dbf96469..8bb1aaf028 100644 --- a/.github/workflows/printer-linter-pr-post.yml +++ b/.github/workflows/printer-linter-pr-post.yml @@ -39,6 +39,11 @@ jobs: echo "pr_id=$(cat printer-linter-result/pr-id.txt)" >> $GITHUB_ENV echo "pr_head_repo=$(cat printer-linter-result/pr-head-repo.txt)" >> $GITHUB_ENV echo "pr_head_ref=$(cat printer-linter-result/pr-head-ref.txt)" >> $GITHUB_ENV + if [[ -f "printer-linter-result/comment.md" ]]; then + echo "commentFileExists=true" >> $GITHUB_ENV + else + echo "commentFileExists=false" >> $GITHUB_ENV + fi - uses: actions/checkout@v3 with: @@ -72,6 +77,13 @@ jobs: mkdir printer-linter-result unzip printer-linter-result.zip -d printer-linter-result + - name: Run PR Comments + if: env.commentFileExists == 'true' + uses: peter-evans/create-or-update-comment@v4 + with: + issue-number: ${{ env.pr_id }} + body-path: 'printer-linter-result/comment.md' + - name: Run clang-tidy-pr-comments action uses: platisd/clang-tidy-pr-comments@bc0bb7da034a8317d54e7fe1e819159002f4cc40 with: diff --git a/.printer-linter b/.printer-linter index 3a42a5c033..9724c63300 100644 --- a/.printer-linter +++ b/.printer-linter @@ -3,6 +3,8 @@ checks: diagnostic-mesh-file-size: true diagnostic-definition-redundant-override: true diagnostic-resources-macos-app-directory-name: true + diagnostic-resource-file-deleted: true + diagnostic-material-temperature-defined: true fixes: diagnostic-definition-redundant-override: true format: diff --git a/cura/Machines/MaterialNode.py b/cura/Machines/MaterialNode.py index 179451ff67..a833ad0470 100644 --- a/cura/Machines/MaterialNode.py +++ b/cura/Machines/MaterialNode.py @@ -21,18 +21,25 @@ class MaterialNode(ContainerNode): Its subcontainers are quality profiles. """ - def __init__(self, container_id: str, variant: "VariantNode") -> None: + def __init__(self, container_id: str, variant: "VariantNode", *, container: ContainerInterface = None) -> None: super().__init__(container_id) self.variant = variant self.qualities = {} # type: Dict[str, QualityNode] # Mapping container IDs to quality profiles. self.materialChanged = Signal() # Triggered when the material is removed or its metadata is updated. container_registry = ContainerRegistry.getInstance() - my_metadata = container_registry.findContainersMetadata(id = container_id)[0] - self.base_file = my_metadata["base_file"] - self.material_type = my_metadata["material"] - self.brand = my_metadata["brand"] - self.guid = my_metadata["GUID"] + + if container is not None: + self.base_file = container.getMetaDataEntry("base_file") + self.material_type = container.getMetaDataEntry("material") + self.brand = container.getMetaDataEntry("brand") + self.guid = container.getMetaDataEntry("GUID") + else: + my_metadata = container_registry.findContainersMetadata(id = container_id)[0] + self.base_file = my_metadata["base_file"] + self.material_type = my_metadata["material"] + self.brand = my_metadata["brand"] + self.guid = my_metadata["GUID"] self._loadAll() container_registry.containerRemoved.connect(self._onRemoved) container_registry.containerMetaDataChanged.connect(self._onMetadataChanged) diff --git a/cura/Machines/VariantNode.py b/cura/Machines/VariantNode.py index 6c3e9359d0..b976841aa7 100644 --- a/cura/Machines/VariantNode.py +++ b/cura/Machines/VariantNode.py @@ -148,7 +148,7 @@ class VariantNode(ContainerNode): if "empty_material" in self.materials: del self.materials["empty_material"] - self.materials[base_file] = MaterialNode(container.getId(), variant = self) + self.materials[base_file] = MaterialNode(container.getId(), variant = self, container = container) self.materials[base_file].materialChanged.connect(self.materialsChanged) self.materialsChanged.emit(self.materials[base_file]) diff --git a/printer-linter/pyproject.toml b/printer-linter/pyproject.toml index c346dc0496..cde196225c 100644 --- a/printer-linter/pyproject.toml +++ b/printer-linter/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "printerlinter" description = "Cura UltiMaker printer linting tool" -version = "0.1.1" +version = "0.1.2" authors = [ { name = "UltiMaker", email = "cura@ultimaker.com" } ] diff --git a/printer-linter/src/printerlinter/diagnostic.py b/printer-linter/src/printerlinter/diagnostic.py index 27f4fdd14a..1ef8bef638 100644 --- a/printer-linter/src/printerlinter/diagnostic.py +++ b/printer-linter/src/printerlinter/diagnostic.py @@ -32,3 +32,13 @@ class Diagnostic: }, "Level": self.level } + +class GitComment: + def __init__(self, comment: str) -> None: + """ + @param comment: The comment text. + """ + self.comment = comment + + def toDict(self) -> Dict[str, Any]: + return self.comment \ No newline at end of file diff --git a/printer-linter/src/printerlinter/factory.py b/printer-linter/src/printerlinter/factory.py index 4473fb9a4e..0d706788fc 100644 --- a/printer-linter/src/printerlinter/factory.py +++ b/printer-linter/src/printerlinter/factory.py @@ -11,7 +11,7 @@ from .linters.directory import Directory def getLinter(file: Path, settings: dict) -> Optional[List[Linter]]: """ Returns a Linter depending on the file format """ if not file.exists(): - return None + return [Directory(file, settings)] if ".inst" in file.suffixes and ".cfg" in file.suffixes: return [Directory(file, settings), Profile(file, settings)] diff --git a/printer-linter/src/printerlinter/linters/defintion.py b/printer-linter/src/printerlinter/linters/defintion.py index c4e955a2a5..2d68e20db9 100644 --- a/printer-linter/src/printerlinter/linters/defintion.py +++ b/printer-linter/src/printerlinter/linters/defintion.py @@ -28,6 +28,10 @@ class Definition(Linter): for check in self.checkRedefineOverride(): yield check + if self._settings["checks"].get("diagnostic-material-temperature-defined", False): + for check in self.checkMaterialTemperature(): + 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 @@ -41,7 +45,7 @@ class Definition(Linter): definition = self._definitions[definition_name] if "overrides" in definition and definition_name not in ("fdmprinter", "fdmextruder"): for key, value_dict in definition["overrides"].items(): - is_redefined, child_key, child_value, parent = self._isDefinedInParent(key, value_dict, definition['inherits']) + is_redefined, child_key, child_value, parent, inherited_by= self._isDefinedInParent(key, value_dict, definition['inherits']) if is_redefined: redefined = re.compile(r'.*(\"' + key + r'\"[\s\:\S]*?)\{[\s\S]*?\},?') found = redefined.search(self._content) @@ -59,12 +63,40 @@ class Definition(Linter): yield Diagnostic( file = self._file, diagnostic_name = "diagnostic-definition-redundant-override", - message = f"Overriding {key} with the same value ({child_key}: {child_value}) as defined in parent definition: {definition['inherits']}", + message = f"Overriding {key} with the same value ({child_key}: {child_value}) as defined in parent definition: {inherited_by}", level = "Warning", offset = found.span(0)[0], replacements = replacements ) + def checkMaterialTemperature(self) -> Iterator[Diagnostic]: + """Checks if definition file has material tremperature defined within them""" + definition_name = list(self._definitions.keys())[0] + definition = self._definitions[definition_name] + if "overrides" in definition and definition_name not in ("fdmprinter", "fdmextruder"): + for key, value_dict in definition["overrides"].items(): + if "temperature" in key and "material" in key: + + redefined = re.compile(r'.*(\"' + key + r'\"[\s\:\S]*?)\{[\s\S]*?\},?') + found = redefined.search(self._content) + if len(found.group().splitlines()) > 1: + replacements = [] + else: + replacements = [Replacement( + file=self._file, + offset=found.span(1)[0], + length=len(found.group()), + replacement_text="")] + + yield Diagnostic( + file=self._file, + diagnostic_name="diagnostic-material-temperature-defined", + message=f"Overriding {key} as it belongs to material temperature catagory and shouldn't be placed in machine definitions", + level="Warning", + offset=found.span(0)[0], + replacements=replacements + ) + def _loadDefinitionFiles(self, definition_file) -> None: """ Loads definition file contents into self._definitions. Also load parent definition if it exists. """ definition_name = Path(definition_file.stem).stem @@ -85,7 +117,7 @@ class Definition(Linter): def _isDefinedInParent(self, key, value_dict, inherits_from): if self._ignore(key, "diagnostic-definition-redundant-override"): - return False, None, None, None + return False, None, None, None, None if "overrides" not in self._definitions[inherits_from]: return self._isDefinedInParent(key, value_dict, self._definitions[inherits_from]["inherits"]) @@ -114,11 +146,11 @@ class Definition(Linter): v = child_value cv = check_value if v == cv: - return True, child_key, child_value, parent + return True, child_key, child_value, parent, inherits_from if "inherits" in parent: return self._isDefinedInParent(key, value_dict, parent["inherits"]) - return False, None, None, None + return False, None, None, None, None def _loadBasePrinterSettings(self): settings = {} diff --git a/printer-linter/src/printerlinter/linters/directory.py b/printer-linter/src/printerlinter/linters/directory.py index 4ca299dad7..e77cac96b9 100644 --- a/printer-linter/src/printerlinter/linters/directory.py +++ b/printer-linter/src/printerlinter/linters/directory.py @@ -1,7 +1,7 @@ from pathlib import Path from typing import Iterator -from ..diagnostic import Diagnostic +from ..diagnostic import Diagnostic, GitComment from .linter import Linter @@ -11,9 +11,12 @@ class Directory(Linter): super().__init__(file, settings) def check(self) -> Iterator[Diagnostic]: - if self._settings["checks"].get("diagnostic-resources-macos-app-directory-name", False): + if self._file.exists() and self._settings["checks"].get("diagnostic-resources-macos-app-directory-name", False): for check in self.checkForDotInDirName(): yield check + elif self._settings["checks"].get("diagnostic-resource-file-deleted", False): + for check in self.checkFilesDeleted(): + yield check yield @@ -29,3 +32,8 @@ class Directory(Linter): ) yield + def checkFilesDeleted(self) -> Iterator[GitComment]: + if not self._file.exists(): + """ Check if there is a file that is deleted, this causes upgrade scripts to not work properly """ + yield GitComment( f'File: **{self._file}** must not be deleted as it is not allowed. It will create issues upgrading Cura' ) + yield \ No newline at end of file diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index fb5ee36bd0..d93372571f 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -19,6 +19,7 @@ def main() -> None: 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("--deleted", action="store_true", help="Check for deleted 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") @@ -41,12 +42,26 @@ def main() -> None: settings = yaml.load(f, yaml.FullLoader) full_body_check = {"Diagnostics": []} + comments_check = {"Error Files": []} for file in files: if not path.exists(file): print(f"Can't find the file: {file}") return + if args.deleted: + for file in args.Files: + if file not in files: + deletedFiles = diagnoseIssuesWithFile(file, settings) + comments_check["Error Files"].extend([d.toDict() for d in deletedFiles]) + + results = yaml.dump(comments_check, default_flow_style=False, indent=4, width=240) + + if report: + report.write_text(results) + else: + print(results) + if to_fix or to_diagnose: for file in files: diagnostics = diagnoseIssuesWithFile(file, settings) @@ -82,7 +97,6 @@ def diagnoseIssuesWithFile(file: Path, settings: dict) -> List[Diagnostic]: return linter_results - def applyFixesToFile(file, settings, full_body_check) -> None: if not file.exists(): return