Merge pull request #10 from YunoHost/fix+pythonic_style

Fix+pythonic style

package_linter.py

@@ -18,10 +18,10 @@ class c:
 
 def header(app_path):
     print(c.UNDERLINE + c.HEADER + c.BOLD +
-          "YUNOHOST APP PACKAGE LINTER\n" + c.END)
+          "YUNOHOST APP PACKAGE LINTER\n", c.END,
-    print("App packaging documentation: https://yunohost.org/#/packaging_apps")
+    "App packaging documentation: https://yunohost.org/#/packaging_apps\n",
-    print("App package example: https://github.com/YunoHost/example_ynh\n")
+    "App package example: https://github.com/YunoHost/example_ynh\n",
-    print("Checking " + c.BOLD + app_path + c.END + " package\n")
+    "Checking " + c.BOLD + app_path + c.END + " package\n")
 
 
 def print_right(str):
@@ -37,15 +37,14 @@ def check_files_exist(app_path):
     Check files exist
     """
     print (c.BOLD + c.HEADER + ">>>> MISSING FILES <<<<" + c.END)
-    fname = ("manifest.json", "scripts/install", "scripts/remove",
+    fnames = ("manifest.json", "scripts/install", "scripts/remove",
              "scripts/upgrade", "scripts/backup", "scripts/restore", "LICENSE", "README.md")
-    i = 0
+
-    while (i < len(fname)):
+    for fname in fnames:
-        if (check_file_exist(app_path + "/" + fname[i])):
+        if check_file_exist(app_path + "/" + fname):
-            print_right(fname[i])
+            print_right(fname)
         else:
-            print_wrong(fname[i])
+            print_wrong(fname)
-        i += 1
 
 
 def check_file_exist(file_path):
@@ -63,6 +62,7 @@ def check_manifest(manifest):
     """
     Check if there is no comma syntax issue
     """
+
     try:
         with open(manifest, encoding='utf-8') as data_file:
             manifest = json.loads(data_file.read())
@@ -71,27 +71,34 @@ def check_manifest(manifest):
         print_wrong(
             "Syntax (comma) or encoding issue with manifest.json. Can't check file.")
         return
-    i, fields = 0, ("name", "id", "packaging_format", "description", "url",
+
+    fields = ("name", "id", "packaging_format", "description", "url",
               "license", "maintainer", "requirements", "multi_instance", "services", "arguments")
-    while (i < len(fields)):
+
-        if fields[i] in manifest:
+    for field in fields:
-            print_right("\"" + fields[i] + "\" field is present")
+        if field in manifest:
+            print_right("\"" + field + "\" field is present")
         else:
-            print_wrong("\"" + fields[i] + "\" field is missing")
+            print_wrong("\"" + field + "\" field is missing")
-        i += 1
+
     """
     Check values in keys
     """
+
     pf = 1
+
     if "packaging_format" not in manifest:
         print_wrong("\"packaging_format\" key is missing")
         pf = 0
+
     if pf == 1 and isinstance(manifest["packaging_format"], int) != 1:
         print_wrong("\"packaging_format\": value isn't an integer type")
         pf = 0
+
     if pf == 1 and manifest["packaging_format"] != 1:
         print_wrong("\"packaging_format\" field: current format value is '1'")
         pf = 0
+
     if pf == 1:
         print_right("\"packaging_format\" field is good")
 
@@ -100,40 +107,43 @@ def check_manifest(manifest):
             "You should specify 'free' or 'non-free' software package in the license field.")
     elif "license" in manifest:
         print_right("\"licence\" key value is good")
+
     if "multi_instance" in manifest and manifest["multi_instance"] != 1 and manifest["multi_instance"] != 0:
         print_wrong(
             "\"multi_instance\" field must be boolean type values 'true' or 'false' and not string type")
     elif "multi_instance" in manifest:
         print_right("\"multi_instance\" field is good")
+
     if "services" in manifest:
         services = ("nginx", "php5-fpm", "mysql", "uwsgi", "metronome",
                     "postfix", "dovecot")  # , "rspamd", "rmilter")
-        i = 0
+
-        while (i < len(manifest["services"])):
+        for service in manifest["services"]:
-            if manifest["services"][i] not in services:
+            if service not in services:
-                print_wrong(manifest["services"][i] + " service doesn't exist")
+                print_wrong(service + " service doesn't exist")
-            i += 1
+
     if "install" in manifest["arguments"]:
         types = ("domain", "path", "password", "user", "admin")
-        i = 0
+
-        while (i < len(types)):
+        for nbr, typ in enumerate(types):
-            j = 0
+            for install_arg in manifest["arguments"]["install"]:
-            while (j < len(manifest["arguments"]["install"])):
+                if typ == install_arg["name"]:
-                if types[i] == manifest["arguments"]["install"][j]["name"]:
+                    if "type" not in install_arg:
-                    if "type" not in manifest["arguments"]["install"][j]:
                         print("You should specify the type of the key with", end=" ")
-                        print(types[i - 1]) if i == 4 else print(types[i])
+                        print(types[nbr - 1]) if nbr == 4 else print(typ)
-                j += 1
-            i += 1
 
 
 def check_script(path, script_name):
     script_path = path + "/scripts/" + script_name
+
     if check_file_exist(script_path) == 0:
         return
+
     print (c.BOLD + c.HEADER + "\n>>>>",
-           scripts[i].upper(), "SCRIPT <<<<" + c.END)
+           script_name.upper(), "SCRIPT <<<<" + c.END)
+
     script = read_file(script_path)
+
     check_script_header_presence(script)
     check_sudo_prefix_commands(script)
     check_verifications_done_before_modifying_system(script)
@@ -151,23 +161,21 @@ def check_sudo_prefix_commands(script):
     """
     Check if commands are prefix with "sudo"
     """
-    cmd = ("rm", "chown", "chmod", "apt-get", "apt",
+    cmds = ("rm", "chown", "chmod", "apt-get", "apt",
            "service", "yunohost", "find" "swapon", "mkswap", "useradd")  # , "dd") cp, mkdir
-    i, ok = 0, 1
+    ok = True
-    while i < len(script):
+
-        j = 0
+    for line_nbr, line in enumerate(script):
-        while j < len(cmd):
+        for cmd in cmds:
-            if cmd[j] + " " in script[i] and "sudo " + cmd[j] + " " not in script[i] \
+            if cmd + " " in line and "sudo " + cmd + " " not in line \
-                    and "yunohost service" not in script[i] and "-exec " + cmd[j] not in script[i] \
+                    and "yunohost service" not in line and "-exec " + cmd not in line \
-                    and ".service" not in script[i] and script[i][0] != '#':
+                    and ".service" not in line and line[0] != '#':
-                print(c.FAIL + "✘ Line ", i + 1,
+                print(c.FAIL + "✘ Line ", line_nbr + 1,
                       "you should add \"sudo\" before this command line:", c.END)
-                print("  " + script[i].replace(cmd[j],
+                print("  " + line.replace(cmd,
-                                               c.BOLD + c.FAIL + cmd[j] + c.END))
+                                       c.BOLD + c.FAIL + cmd + c.END))
-                ok = 0
+                ok = False
-            j += 1
+    if ok:
-        i += 1
-    if ok == 1:
         print_right("All commands are prefix with \"sudo\".")
 
 
@@ -175,28 +183,29 @@ def check_verifications_done_before_modifying_system(script):
     """
     Check if verifications are done before modifying the system
     """
-    ex, i = 0, 0
+    ex = 0
-    while i < len(script):
+    for line_number, line in enumerate(script):
-        if "ynh_die" in script[i] or "exit" in script[i]:
+        if "ynh_die" in line or "exit" in line:
-            ex = i
+            ex = line_number
-        i += 1
+
-    cmd = ("cp", "mkdir", "rm", "chown", "chmod", "apt-get", "apt", "service",
+    cmds = ("cp", "mkdir", "rm", "chown", "chmod", "apt-get", "apt", "service",
            "find", "sed", "mysql", "swapon", "mount", "dd", "mkswap", "useradd")  # "yunohost"
-    i, ok = 0, 1
+
-    while i < len(script):
+    ok = True
-        if i >= ex:
+
+    for line_nbr, line in enumerate(script):
+        if line_nbr >= ex:
             break
-        j = 0
+
-        while (j < len(cmd)):
+        for cmd in cmds:
-            if cmd[j] in script[i] and script[i][0] != '#':
+            if cmd in line and line[0] != '#':
-                ok = 0
+                ok = False
-            j += 1
+
-        i += 1
+    if not ok:
-    if ok == 0:
         print(c.FAIL + "✘ At line", ex + 1,
-              "'ynh_die' or 'exit' command is executed with system modification before.")
+              "'ynh_die' or 'exit' command is executed with system modification before.\n",
-        print("This system modification is an issue if a verification exit the script.")
+        "This system modification is an issue if a verification exit the script.\n",
-        print("You should move this verification before any system modification." + c.END)
+        "You should move this verification before any system modification." + c.END)
     else:
         print_right(
             "Verifications (with 'ynh_die' or 'exit' commands) are done before any system modification.")
@@ -207,40 +216,45 @@ def check_non_helpers_usage(script):
     - 'yunohost app setting' –> ynh_app_setting_(set,get,delete)
     - 'exit' –> 'ynh_die'
     """
-    i, ok = 0, 1
+    ok = True
-    while i < len(script):
+
-        if "yunohost app setting" in script[i]:
+    for line_nbr, line in enumerate(script):
-            print_wrong("Line {}: 'yunohost app setting' command is deprecated, please use helpers ynh_app_setting_(set,get,delete).".format(i + 1))
+        if "yunohost app setting" in line:
-            ok = 0
+            print_wrong("Line {}: 'yunohost app setting' command is deprecated, please use helpers ynh_app_setting_(set,get,delete).".format(line_nbr + 1))
-        i += 1
+            ok = False
-    if ok == 1:
+
+    if ok:
         print_right("Only helpers are used")
     else:
         print_wrong("Helpers documentation: https://yunohost.org/#/packaging_apps_helpers\ncode: https://github.com/YunoHost/yunohost/…helpers")
 
-    i, ok = 0, 1
+    ok = True
-    while i < len(script):
+
-        if "exit" in script[i]:
+    for line_nbr, line in enumerate(script):
-            print_wrong("Line {}: 'exit' command shouldn't be used. Use 'ynh_die' helper instead.".format(i + 1))
+        if "exit" in line:
-            ok = 0
+            print_wrong("Line {}: 'exit' command shouldn't be used. Use 'ynh_die' helper instead.".format(line_nbr + 1))
-        i += 1
+            ok = False
-    if ok == 1:
+
+    if ok:
         print_right("no 'exit' command found: 'ynh_die' helper is possibly used")
 
 if __name__ == '__main__':
     os.system("clear")
+
     if len(sys.argv) != 2:
         print("Give one app package path.")
         exit()
+
     app_path = sys.argv[1]
     header(app_path)
     check_files_exist(app_path)
     check_manifest(app_path + "/manifest.json")
-    i, scripts = 0, ["install", "remove", "upgrade", "backup", "restore"]
+
+    scripts = ["install", "remove", "upgrade", "backup", "restore"]
     for (dirpath, dirnames, filenames) in os.walk(os.path.join(app_path, "scripts")):
         for filename in filenames:
             if filename not in scripts and filename[-4:] != ".swp":
                 scripts.append(filename)
-    while i < len(scripts):
+
-        check_script(app_path, scripts[i])
+    for script in scripts:
-        i += 1
+        check_script(app_path, script)