chiark / gitweb /
Switch all headers to python3
[fdroidserver.git] / fdroidserver / lint.py
index ad6f4f5c159a628975fc60f97feac4dfd3ae794b..f02bb7ed98403c4438a995b912ed301bcafaf644 100644 (file)
@@ -1,5 +1,4 @@
-#!/usr/bin/env python2
-# -*- coding: utf-8 -*-
+#!/usr/bin/env python3
 #
 # lint.py - part of the FDroid server tool
 # Copyright (C) 2013-2014 Daniel Martí <mvdan@mvdan.cc>
 
 from argparse import ArgumentParser
 import re
-import common
-import metadata
 import sys
 from sets import Set
 
+import common
+import metadata
+import rewritemeta
+
 config = None
 options = None
 
@@ -55,26 +56,30 @@ http_url_shorteners = [
 http_checks = https_enforcings + http_url_shorteners + [
     (re.compile(r'.*github\.com/[^/]+/[^/]+\.git'),
      "Appending .git is not necessary"),
-    (re.compile(r'(.*/blob/master/|.*raw\.github.com/[^/]*/[^/]*/master/)'),
-     "Use /HEAD/ instead of /master/ to point at a file in the default branch"),
+    (re.compile(r'.*://[^/]*(github|gitlab|bitbucket|rawgit)[^/]*/([^/]+/){1,3}master'),
+     "Use /HEAD instead of /master to point at a file in the default branch"),
 ]
 
 regex_checks = {
-    'Web Site': http_checks + [
-    ],
-    'Source Code': http_checks + [
-    ],
-    'Repo': https_enforcings + [
-    ],
+    'Web Site': http_checks,
+    'Source Code': http_checks,
+    'Repo': https_enforcings,
     'Issue Tracker': http_checks + [
-        (re.compile(r'.*github\.com/[^/]+/[^/]+[/]*$'),
+        (re.compile(r'.*github\.com/[^/]+/[^/]+/*$'),
+         "/issues is missing"),
+        (re.compile(r'.*gitlab\.com/[^/]+/[^/]+/*$'),
          "/issues is missing"),
     ],
     'Donate': http_checks + [
         (re.compile(r'.*flattr\.com'),
          "Flattr donation methods belong in the FlattrID flag"),
     ],
-    'Changelog': http_checks + [
+    'Changelog': http_checks,
+    'Author Name': [
+        (re.compile(r'^\s'),
+         "Unnecessary leading space"),
+        (re.compile(r'.*\s$'),
+         "Unnecessary trailing space"),
     ],
     'License': [
         (re.compile(r'^(|None|Unknown)$'),
@@ -89,6 +94,10 @@ regex_checks = {
          "No need to specify that the app is for Android"),
         (re.compile(r'.*[a-z0-9][.!?]( |$)'),
          "Punctuation should be avoided"),
+        (re.compile(r'^\s'),
+         "Unnecessary leading space"),
+        (re.compile(r'.*\s$'),
+         "Unnecessary trailing space"),
     ],
     'Description': [
         (re.compile(r'^No description available$'),
@@ -101,7 +110,7 @@ regex_checks = {
          "Unnecessary trailing space"),
         (re.compile(r'.*([^[]|^)\[[^:[\]]+( |\]|$)'),
          "Invalid link - use [http://foo.bar Link title] or [http://foo.bar]"),
-        (re.compile(r'.*[^[]https?://[^ ]+'),
+        (re.compile(r'(^|.* )https?://[^ ]+'),
          "Unlinkified link - use [http://foo.bar Link title] or [http://foo.bar]"),
     ],
 }
@@ -110,56 +119,55 @@ regex_checks = {
 def check_regexes(app):
     for f, checks in regex_checks.iteritems():
         for m, r in checks:
-            v = app[f]
-            if type(v) == str:
+            v = app.get_field(f)
+            t = metadata.fieldtype(f)
+            if t == metadata.TYPE_MULTILINE:
+                for l in v.splitlines():
+                    if m.match(l):
+                        yield "%s at line '%s': %s" % (f, l, r)
+            else:
                 if v is None:
                     continue
                 if m.match(v):
                     yield "%s '%s': %s" % (f, v, r)
-            elif type(v) == list:
-                for l in v:
-                    if m.match(l):
-                        yield "%s at line '%s': %s" % (f, l, r)
 
 
 def get_lastbuild(builds):
     lowest_vercode = -1
     lastbuild = None
     for build in builds:
-        if not build['disable']:
-            vercode = int(build['vercode'])
+        if not build.disable:
+            vercode = int(build.vercode)
             if lowest_vercode == -1 or vercode < lowest_vercode:
                 lowest_vercode = vercode
-        if not lastbuild or int(build['vercode']) > int(lastbuild['vercode']):
+        if not lastbuild or int(build.vercode) > int(lastbuild.vercode):
             lastbuild = build
     return lastbuild
 
 
 def check_ucm_tags(app):
-    lastbuild = get_lastbuild(app['builds'])
+    lastbuild = get_lastbuild(app.builds)
     if (lastbuild is not None
-            and lastbuild['commit']
-            and app['Update Check Mode'] == 'RepoManifest'
-            and not lastbuild['commit'].startswith('unknown')
-            and lastbuild['vercode'] == app['Current Version Code']
-            and not lastbuild['forcevercode']
-            and any(s in lastbuild['commit'] for s in '.,_-/')):
+            and lastbuild.commit
+            and app.UpdateCheckMode == 'RepoManifest'
+            and not lastbuild.commit.startswith('unknown')
+            and lastbuild.vercode == app.CurrentVersionCode
+            and not lastbuild.forcevercode
+            and any(s in lastbuild.commit for s in '.,_-/')):
         yield "Last used commit '%s' looks like a tag, but Update Check Mode is '%s'" % (
-            lastbuild['commit'], app['Update Check Mode'])
+            lastbuild.commit, app.UpdateCheckMode)
 
 
 def check_char_limits(app):
     limits = config['char_limits']
 
-    summ_chars = len(app['Summary'])
-    if summ_chars > limits['Summary']:
+    if len(app.Summary) > limits['Summary']:
         yield "Summary of length %s is over the %i char limit" % (
-            summ_chars, limits['Summary'])
+            len(app.Summary), limits['Summary'])
 
-    desc_charcount = sum(len(l) for l in app['Description'])
-    if desc_charcount > limits['Description']:
+    if len(app.Description) > limits['Description']:
         yield "Description of length %s is over the %i char limit" % (
-            desc_charcount, limits['Description'])
+            len(app.Description), limits['Description'])
 
 
 def check_old_links(app):
@@ -172,31 +180,28 @@ def check_old_links(app):
         'gitorious.org',
         'code.google.com',
     ]
-    if any(s in app['Repo'] for s in usual_sites):
+    if any(s in app.Repo for s in usual_sites):
         for f in ['Web Site', 'Source Code', 'Issue Tracker', 'Changelog']:
-            if any(s in app[f] for s in old_sites):
-                yield "App is in '%s' but has a link to '%s'" % (app['Repo'], app[f])
+            v = app.get_field(f)
+            if any(s in v for s in old_sites):
+                yield "App is in '%s' but has a link to '%s'" % (app.Repo, v)
 
 
 def check_useless_fields(app):
-    if app['Update Check Name'] == app['id']:
+    if app.UpdateCheckName == app.id:
         yield "Update Check Name is set to the known app id - it can be removed"
 
-filling_ucms = re.compile('^(Tags.*|RepoManifest.*)')
+filling_ucms = re.compile(r'^(Tags.*|RepoManifest.*)')
 
 
 def check_checkupdates_ran(app):
-    if filling_ucms.match(app['Update Check Mode']):
-        if all(app[f] == metadata.app_defaults[f] for f in [
-                'Auto Name',
-                'Current Version',
-                'Current Version Code',
-                ]):
+    if filling_ucms.match(app.UpdateCheckMode):
+        if not app.AutoName and not app.CurrentVersion and app.CurrentVersionCode == '0':
             yield "UCM is set but it looks like checkupdates hasn't been run yet"
 
 
 def check_empty_fields(app):
-    if not app['Categories']:
+    if not app.Categories:
         yield "Categories are not set"
 
 all_categories = Set([
@@ -221,31 +226,37 @@ all_categories = Set([
 
 
 def check_categories(app):
-    for categ in app['Categories']:
+    for categ in app.Categories:
         if categ not in all_categories:
             yield "Category '%s' is not valid" % categ
 
 
 def check_duplicates(app):
-    if app['Web Site'] and app['Source Code']:
-        if app['Web Site'].lower() == app['Source Code'].lower():
-            yield "Website '%s' is just the app's source code link" % app['Web Site']
+    if app.Name and app.Name == app.AutoName:
+        yield "Name '%s' is just the auto name - remove it" % app.Name
 
-    if app['Name'] and app['Name'] == app['Auto Name']:
-        yield "Name '%s' is just the auto name" % app['Name']
+    links_seen = set()
+    for f in ['Source Code', 'Web Site', 'Issue Tracker', 'Changelog']:
+        v = app.get_field(f)
+        if not v:
+            continue
+        v = v.lower()
+        if v in links_seen:
+            yield "Duplicate link in '%s': %s" % (f, v)
+        else:
+            links_seen.add(v)
 
-    name = app['Name'] or app['Auto Name']
-    if app['Summary'] and name:
-        if app['Summary'].lower() == name.lower():
-            yield "Summary '%s' is just the app's name" % app['Summary']
+    name = app.Name or app.AutoName
+    if app.Summary and name:
+        if app.Summary.lower() == name.lower():
+            yield "Summary '%s' is just the app's name" % app.Summary
 
-    desc = app['Description']
-    if app['Summary'] and desc and len(desc) == 1:
-        if app['Summary'].lower() == desc[0].lower():
-            yield "Description '%s' is just the app's summary" % app['Summary']
+    if app.Summary and app.Description and len(app.Description) == 1:
+        if app.Summary.lower() == app.Description[0].lower():
+            yield "Description '%s' is just the app's summary" % app.Summary
 
     seenlines = set()
-    for l in app['Description']:
+    for l in app.Description.splitlines():
         if len(l) < 1:
             continue
         if l in seenlines:
@@ -253,11 +264,11 @@ def check_duplicates(app):
         seenlines.add(l)
 
 
-desc_url = re.compile("[^[]\[([^ ]+)( |\]|$)")
+desc_url = re.compile(r'(^|[^[])\[([^ ]+)( |\]|$)')
 
 
 def check_mediawiki_links(app):
-    wholedesc = ' '.join(app['Description'])
+    wholedesc = ' '.join(app.Description)
     for um in desc_url.finditer(wholedesc):
         url = um.group(1)
         for m, r in http_checks:
@@ -269,7 +280,7 @@ def check_bulleted_lists(app):
     validchars = ['*', '#']
     lchar = ''
     lcount = 0
-    for l in app['Description']:
+    for l in app.Description.splitlines():
         if len(l) < 1:
             lcount = 0
             continue
@@ -285,16 +296,20 @@ def check_bulleted_lists(app):
 
 
 def check_builds(app):
-    for build in app['builds']:
-        if build['disable']:
+    for build in app.builds:
+        if build.disable:
+            if build.disable.startswith('Generated by import.py'):
+                yield "Build generated by `fdroid import` - remove disable line once ready"
             continue
         for s in ['master', 'origin', 'HEAD', 'default', 'trunk']:
-            if build['commit'] and build['commit'].startswith(s):
-                yield "Branch '%s' used as commit in build '%s'" % (s, build['version'])
-            for srclib in build['srclibs']:
+            if build.commit and build.commit.startswith(s):
+                yield "Branch '%s' used as commit in build '%s'" % (s, build.version)
+            for srclib in build.srclibs:
                 ref = srclib.split('@')[1].split('/')[0]
                 if ref.startswith(s):
                     yield "Branch '%s' used as commit in srclib '%s'" % (s, srclib)
+        if build.target and build.build_method() == 'gradle':
+            yield "target= has no gradle support"
 
 
 def main():
@@ -305,11 +320,10 @@ def main():
 
     # Parse command line...
     parser = ArgumentParser(usage="%(prog)s [options] [APPID [APPID ...]]")
+    common.setup_global_opts(parser)
+    parser.add_argument("-f", "--format", action="store_true", default=False,
+                        help="Also warn about formatting issues, like rewritemeta -l")
     parser.add_argument("appid", nargs='*', help="app-id in the form APPID")
-    parser.add_argument("-v", "--verbose", action="store_true", default=False,
-                        help="Spew out even more information than normal")
-    parser.add_argument("-q", "--quiet", action="store_true", default=False,
-                        help="Restrict output to warnings and errors")
     options = parser.parse_args()
 
     config = common.read_config(options)
@@ -319,7 +333,7 @@ def main():
     apps = common.read_app_args(options.appid, allapps, False)
 
     for appid, app in apps.iteritems():
-        if app['Disabled']:
+        if app.Disabled:
             continue
 
         warns = []
@@ -340,10 +354,14 @@ def main():
                 ]:
             warns += check_func(app)
 
+        if options.format:
+            if not rewritemeta.proper_format(app):
+                warns.append("Run rewritemeta to fix formatting")
+
         if warns:
             anywarns = True
             for warn in warns:
-                print "%s: %s" % (appid, warn)
+                print("%s: %s" % (appid, warn))
 
     if anywarns:
         sys.exit(1)