Update the check-bot (devicons#970)

* Add check for strokes in check-bot

* Add check for svg file name

* Update gulpfile to remove x and y of svg elem

* Revert name changes to err file

* check-bot now check devicon object as well

* Fixed minor bugs

* Improve logging in check_icon

Co-authored-by: David Leal <halfpacho@gmail.com>

.github/scripts/build_assets/arg_getters.py

@@ -34,28 +34,32 @@ def get_selenium_runner_args(peek_mode=False):
                         action=PathResolverAction)
 
     if peek_mode:
-        parser.add_argument("--pr_title",
+        parser.add_argument("pr_title",
                             help="The title of the PR that we are peeking at")
     else:
         parser.add_argument("token",
-                            help="The GitHub token to access the GitHub REST API.",
-                            type=str)
+                            help="The GitHub token to access the GitHub REST API.")
 
     return parser.parse_args()
 
 
-def get_check_svgs_on_pr_args():
+def get_check_icon_pr_args():
     """
-    Get the commandline arguments for the check_svgs_on_pr.py.
+    Get the commandline arguments for the check_icon_pr.py.
     """
     parser = ArgumentParser(description="Check the SVGs to ensure their attributes are correct. Run whenever a PR is opened")
-    parser.add_argument("files_added_json_path",
-                        help="The path to the files_added.json created by the gh-action-get-changed-files@2.1.4",
+
+    parser.add_argument("pr_title",
+                        help="The title of the PR that we are peeking at")
+
+    parser.add_argument("icons_folder_path",
+                        help="The path to the icons folder",
                         action=PathResolverAction)
 
-    parser.add_argument("files_modified_json_path",
-                        help="The path to the files_modified.json created by the gh-action-get-changed-files@2.1.4",
+    parser.add_argument("devicon_json_path",
+                        help="The path to the devicon.json",
                         action=PathResolverAction)
+
     return parser.parse_args()
 
 
@@ -65,8 +69,7 @@ def get_release_message_args():
     """
     parser = ArgumentParser(description="Create a text containing the icons and features added since last release.")
     parser.add_argument("token",
-                        help="The GitHub token to access the GitHub REST API.",
-                        type=str)
+                        help="The GitHub token to access the GitHub REST API.")
     return parser.parse_args()
 
 

.github/scripts/build_assets/filehandler.py

@@ -68,7 +68,7 @@ def get_svgs_paths(new_icons: List[dict], icons_folder_path: str,
         folder_path = Path(icons_folder_path, icon_info['name'])
 
         if not folder_path.is_dir():
-            raise ValueError(f"Invalid path. This is not a directory: {folder_path}.")
+            raise ValueError(f"Invalid path. This is not a directory: '{folder_path}'.")
 
         if icon_versions_only:
             get_icon_svgs_paths(folder_path, icon_info, file_paths, as_str)
@@ -100,7 +100,7 @@ def get_icon_svgs_paths(folder_path: Path, icon_info: dict,
         if path.exists():
             file_paths.append(str(path) if as_str else path)
         else:
-            raise ValueError(f"This path doesn't exist: {path}")
+            raise ValueError(f"This path doesn't exist: '{path}'")
 
 
 def get_all_svgs_paths(folder_path: Path, icon_info: dict,
@@ -119,7 +119,7 @@ def get_all_svgs_paths(folder_path: Path, icon_info: dict,
         if path.exists():
             file_paths.append(str(path) if as_str else path)
         else:
-            raise ValueError(f"This path doesn't exist: {path}")
+            raise ValueError(f"This path doesn't exist: '{path}'")
 
 
 def is_alias(font_version: str, aliases: List[dict]):
@@ -207,6 +207,14 @@ def create_screenshot_folder(dir, screenshot_name: str="screenshots/"):
     finally:
         return str(screenshot_folder)
 
+def write_to_file(path: str, value: any):
+    """
+    Write the value to a file.
+    """
+    with open(path, "w") as file:
+        file.write(value)
+
+# --- NOT USED CURRENTLY --- 
 def get_added_modified_svgs(files_added_json_path: str,
     files_modified_json_path: str):
     """
@@ -230,11 +238,3 @@ def get_added_modified_svgs(files_added_json_path: str,
             svgs.append(path)
 
     return svgs
-
-
-def write_to_file(path: str, value: any):
-    """
-    Write the value to a file.
-    """
-    with open(path, "w") as file:
-        file.write(value)

.github/scripts/build_assets/util.py

@@ -5,7 +5,6 @@
 import sys
 import traceback
 
-
 def exit_with_err(err: Exception):
     """
     Exit the current step and display the err.
@@ -66,3 +65,9 @@ def find_object_added_in_pr(icons: List[dict], pr_title: str):
         message = "util.find_object_added_in_pr: Couldn't find an icon matching the name in the PR title.\n" \
             f"PR title is: '{pr_title}'"
         raise Exception(message)
+
+
+valid_svg_filename_pattern = re.compile(r"-(original|plain|line)(-wordmark)?\.svg$")
+def is_svg_name_valid(filename: str):
+    return valid_svg_filename_pattern.search(filename) is not None
+

.github/scripts/check_icon_pr.py

@@ -0,0 +1,166 @@
+from typing import List
+import xml.etree.ElementTree as et
+from pathlib import Path
+
+
+# pycharm complains that build_assets is an unresolved ref
+# don't worry about it, the script still runs
+from build_assets import filehandler, arg_getters, util
+
+
+def main():
+    """
+    Check the quality of the svgs IF this is an icon PR. Else, does nothing.
+    If any svg error is found, create a json file called 'svg_err_messages.json'
+    in the root folder that will contains the error messages.
+    """
+    args = arg_getters.get_check_icon_pr_args()
+    try:
+        all_icons = filehandler.get_json_file_content(args.devicon_json_path)
+
+        # get only the icon object that has the name matching the pr title
+        filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title)
+        print("Checking devicon.json object: " + str(filtered_icon))
+        devicon_err_msg = check_devicon_object(filtered_icon)
+
+        # check the file names
+        filename_err_msg = ""
+        svgs = None
+        try:
+            svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path, as_str=False)
+            print("SVGs to check: ", *svgs, sep='\n')
+        except ValueError as e:
+            filename_err_msg = "Error found regarding filenames:\n- " + e.args[0]
+
+        # check the svgs
+        if svgs is None or len(svgs) == 0:
+            print("No SVGs to check, ending script.")
+            svg_err_msg = "Error checking SVGs: no SVGs to check. Might be caused by above issues."
+        else:
+            svg_err_msg = check_svgs(svgs)
+
+        err_msg = []
+        if devicon_err_msg != "":
+            err_msg.append(devicon_err_msg)
+
+        if filename_err_msg != "":
+            err_msg.append(filename_err_msg)
+
+        if svg_err_msg != "":
+            err_msg.append(svg_err_msg)
+
+        filehandler.write_to_file("./err_messages.txt", "\n\n".join(err_msg))
+        print("Task completed.")
+    except Exception as e:
+        filehandler.write_to_file("./err_messages.txt", str(e))
+        util.exit_with_err(e)
+
+
+def check_devicon_object(icon: dict):
+    """
+    Check that the devicon object added is up to standard.
+    :return a string containing the error messages if any.
+    """
+    err_msgs = []
+    try:
+        for tag in icon["tags"]:
+            if type(tag) != str:
+                raise TypeError()
+    except TypeError:
+        err_msgs.append("- 'tags' must be an array of strings, not: " + str(icon["tags"]))
+    except KeyError:
+        err_msgs.append("- missing key: 'tags'.")
+
+    try:
+        if type(icon["versions"]) != dict:
+            err_msgs.append("- 'versions' must be an object.")
+    except KeyError:
+        err_msgs.append("- missing key: 'versions'.")
+
+    try:
+        if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0:
+            err_msgs.append("- must contain at least 1 svg version in a list.")
+
+        for version in icon["versions"]["svg"]:
+            if not util.is_version_name_valid(version):
+                err_msgs.append(f"- Invalid version name in versions['svg']: '{version}'. Must match regexp: (original|plain|line)(-wordmark)?")
+    except KeyError:
+        err_msgs.append("- missing key: 'svg' in 'versions'.")
+
+    try:
+        if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0:
+            err_msgs.append("- must contain at least 1 font version in a list.")
+
+        for version in icon["versions"]["font"]:
+            if not util.is_version_name_valid(version):
+                err_msgs.append(f"- Invalid version name in versions['font']: '{version}'. Must match regexp: (original|plain|line)(-wordmark)?")
+    except KeyError:
+        err_msgs.append("- missing key: 'font' in 'versions'.")
+
+    try:
+        if type(icon["color"]) != str or "#" not in icon["color"]:
+            err_msgs.append("- 'color' must be a string in the format '#abcdef'")
+    except KeyError:
+        err_msgs.append("- missing key: 'color'.")
+
+    try:
+        if type(icon["aliases"]) != list:
+            err_msgs.append("- 'aliases' must be an array.")
+    except KeyError:
+        err_msgs.append("- missing key: 'aliases'.")
+
+    if len(err_msgs) > 0:
+        message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs))
+        return message
+    return "" 
+
+
+def check_svgs(svg_file_paths: List[Path]):
+    """
+    Check the width, height, viewBox and style of each svgs passed in.
+    The viewBox must be '0 0 128 128'.
+    The style must not contain any 'stroke' declarations.
+    If any error is found, they will be thrown.
+    :param: svg_file_paths, the file paths to the svg to check for.
+    :return: None if there no errors. If there is, return a JSON.stringified
+    list with the error messages in it.
+    """
+    # batch err messages together so user can fix everything at once
+    err_msgs = []
+    for svg_path in svg_file_paths:
+        try:
+            err_msg = [f"SVG Error in '{svg_path.name}':"]
+
+            # name check
+            if not util.is_svg_name_valid(svg_path.name):
+                err_msg.append("- SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`")
+
+            # svg check
+            tree = et.parse(svg_path)
+            root = tree.getroot()
+            namespace = "{http://www.w3.org/2000/svg}"
+
+            if root.tag != f"{namespace}svg":
+                err_msg.append(f"- root is '{root.tag}'. Root must be an 'svg' element")
+
+            if root.get("viewBox") != "0 0 128 128":
+                err_msg.append("- 'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.")
+
+            # goes through all elems and check for strokes
+            for child in tree.iter():
+                if child.get("stroke") != None:
+                    err_msg.append("- SVG contains `stroke` property. This will get ignored by Icomoon. Please convert them to fills.")
+                    break
+
+            if len(err_msg) > 1:
+                err_msgs.append("\n".join(err_msg))
+        except et.ParseError as e:
+            raise Exception(f"SVG Error in file: {svg_path}. Full Error: \n" + str(e))
+
+    if len(err_msgs) > 0:
+        return "\n\n".join(err_msgs)
+    return ""
+
+
+if __name__ == "__main__":
+    main()