chiark / gitweb /
Switch all headers to python3
[fdroidserver.git] / fdroidserver / lint.py
index ec4373dd6e547aae4e15a8b5a2e28902e749501a..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>
 # You should have received a copy of the GNU Affero General Public Licen
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from optparse import OptionParser
+from argparse import ArgumentParser
 import re
-import logging
+import sys
+from sets import Set
+
 import common
 import metadata
-from collections import Counter
+import rewritemeta
 
 config = None
 options = None
 
 
 def enforce_https(domain):
-    return (re.compile(r'.*[^sS]://[^/]*' + re.escape(domain) + r'/.*'),
+    return (re.compile(r'.*[^sS]://[^/]*' + re.escape(domain) + r'(/.*)?'),
             domain + " URLs should always use https://")
 
 https_enforcings = [
     enforce_https('github.com'),
     enforce_https('gitlab.com'),
-    enforce_https('gitorious.org'),
+    enforce_https('bitbucket.org'),
     enforce_https('apache.org'),
     enforce_https('google.com'),
     enforce_https('svn.code.sf.net'),
-    enforce_https('googlecode.com'),
 ]
 
-http_warnings = https_enforcings + [
+
+def forbid_shortener(domain):
+    return (re.compile(r'https?://[^/]*' + re.escape(domain) + r'/.*'),
+            "URL shorteners should not be used")
+
+http_url_shorteners = [
+    forbid_shortener('goo.gl'),
+    forbid_shortener('t.co'),
+    forbid_shortener('ur1.ca'),
+]
+
+http_checks = https_enforcings + http_url_shorteners + [
     (re.compile(r'.*github\.com/[^/]+/[^/]+\.git'),
      "Appending .git is not necessary"),
-    # TODO enable in August 2015, when Google Code goes read-only
-    # (re.compile(r'.*://code\.google\.com/.*'),
-    #  "code.google.com will be soon switching down, perhaps the project moved to github.com?"),
+    (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_warnings = {
-    'Web Site': http_warnings + [
-    ],
-    'Source Code': http_warnings + [
-    ],
-    'Repo': https_enforcings + [
-    ],
-    'Issue Tracker': http_warnings + [
-        (re.compile(r'.*github\.com/[^/]+/[^/]+[/]*$'),
+regex_checks = {
+    'Web Site': http_checks,
+    'Source Code': http_checks,
+    'Repo': https_enforcings,
+    'Issue Tracker': http_checks + [
+        (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_warnings + [
+    '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)$'),
@@ -76,6 +94,10 @@ regex_warnings = {
          "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$'),
@@ -86,140 +108,264 @@ regex_warnings = {
          "Unnecessary leading space"),
         (re.compile(r'.*\s$'),
          "Unnecessary trailing space"),
+        (re.compile(r'.*([^[]|^)\[[^:[\]]+( |\]|$)'),
+         "Invalid link - use [http://foo.bar Link title] or [http://foo.bar]"),
+        (re.compile(r'(^|.* )https?://[^ ]+'),
+         "Unlinkified link - use [http://foo.bar Link title] or [http://foo.bar]"),
     ],
 }
 
 
-def main():
+def check_regexes(app):
+    for f, checks in regex_checks.iteritems():
+        for m, r in checks:
+            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)
+
+
+def get_lastbuild(builds):
+    lowest_vercode = -1
+    lastbuild = None
+    for build in builds:
+        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):
+            lastbuild = build
+    return lastbuild
+
+
+def check_ucm_tags(app):
+    lastbuild = get_lastbuild(app.builds)
+    if (lastbuild is not None
+            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.UpdateCheckMode)
+
+
+def check_char_limits(app):
+    limits = config['char_limits']
+
+    if len(app.Summary) > limits['Summary']:
+        yield "Summary of length %s is over the %i char limit" % (
+            len(app.Summary), limits['Summary'])
+
+    if len(app.Description) > limits['Description']:
+        yield "Description of length %s is over the %i char limit" % (
+            len(app.Description), limits['Description'])
+
+
+def check_old_links(app):
+    usual_sites = [
+        'github.com',
+        'gitlab.com',
+        'bitbucket.org',
+    ]
+    old_sites = [
+        'gitorious.org',
+        'code.google.com',
+    ]
+    if any(s in app.Repo for s in usual_sites):
+        for f in ['Web Site', 'Source Code', 'Issue Tracker', 'Changelog']:
+            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.UpdateCheckName == app.id:
+        yield "Update Check Name is set to the known app id - it can be removed"
+
+filling_ucms = re.compile(r'^(Tags.*|RepoManifest.*)')
+
+
+def check_checkupdates_ran(app):
+    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:
+        yield "Categories are not set"
+
+all_categories = Set([
+    "Connectivity",
+    "Development",
+    "Games",
+    "Graphics",
+    "Internet",
+    "Money",
+    "Multimedia",
+    "Navigation",
+    "Phone & SMS",
+    "Reading",
+    "Science & Education",
+    "Security",
+    "Sports & Health",
+    "System",
+    "Theming",
+    "Time",
+    "Writing",
+])
+
+
+def check_categories(app):
+    for categ in app.Categories:
+        if categ not in all_categories:
+            yield "Category '%s' is not valid" % categ
+
+
+def check_duplicates(app):
+    if app.Name and app.Name == app.AutoName:
+        yield "Name '%s' is just the auto name - remove it" % 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.AutoName
+    if app.Summary and name:
+        if app.Summary.lower() == name.lower():
+            yield "Summary '%s' is just the app's name" % 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.splitlines():
+        if len(l) < 1:
+            continue
+        if l in seenlines:
+            yield "Description has a duplicate line"
+        seenlines.add(l)
+
+
+desc_url = re.compile(r'(^|[^[])\[([^ ]+)( |\]|$)')
+
 
-    global config, options, curid, count
-    curid = None
+def check_mediawiki_links(app):
+    wholedesc = ' '.join(app.Description)
+    for um in desc_url.finditer(wholedesc):
+        url = um.group(1)
+        for m, r in http_checks:
+            if m.match(url):
+                yield "URL '%s' in Description: %s" % (url, r)
 
-    count = Counter()
 
-    def warn(message):
-        global curid, count
-        if curid:
-            print "%s:" % curid
-            curid = None
-            count['app'] += 1
-        print '    %s' % message
-        count['warn'] += 1
+def check_bulleted_lists(app):
+    validchars = ['*', '#']
+    lchar = ''
+    lcount = 0
+    for l in app.Description.splitlines():
+        if len(l) < 1:
+            lcount = 0
+            continue
+
+        if l[0] == lchar and l[1] == ' ':
+            lcount += 1
+            if lcount > 2 and lchar not in validchars:
+                yield "Description has a list (%s) but it isn't bulleted (*) nor numbered (#)" % lchar
+                break
+        else:
+            lchar = l[0]
+            lcount = 1
+
+
+def check_builds(app):
+    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:
+                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 pwarn(message):
-        if options.pedantic:
-            warn(message)
+def main():
+
+    global config, options
+
+    anywarns = False
 
     # Parse command line...
-    parser = OptionParser(usage="Usage: %prog [options] [APPID [APPID ...]]")
-    parser.add_option("-v", "--verbose", action="store_true", default=False,
-                      help="Spew out even more information than normal")
-    parser.add_option("-q", "--quiet", action="store_true", default=False,
-                      help="Restrict output to warnings and errors")
-    parser.add_option("-p", "--pedantic", action="store_true", default=False,
-                      help="Show pedantic warnings that might give false positives")
-    (options, args) = parser.parse_args()
+    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")
+    options = parser.parse_args()
 
     config = common.read_config(options)
 
     # Get all apps...
-    allapps = metadata.read_metadata(xref=False)
-    apps = common.read_app_args(args, allapps, False)
+    allapps = metadata.read_metadata(xref=True)
+    apps = common.read_app_args(options.appid, allapps, False)
 
     for appid, app in apps.iteritems():
-        if app['Disabled']:
+        if app.Disabled:
             continue
 
-        curid = appid
-        count['app_total'] += 1
-
-        curbuild = None
-        for build in app['builds']:
-            if not curbuild or int(build['vercode']) > int(curbuild['vercode']):
-                curbuild = build
-
-        # Potentially incorrect UCM
-        if (curbuild and curbuild['commit']
-                and app['Update Check Mode'] == 'RepoManifest'
-                and curbuild['commit'] != 'unknown - see disabled'
-                and any(s in curbuild['commit'] for s in '.,_-/')):
-            pwarn("Last used commit '%s' looks like a tag, but Update Check Mode is '%s'" % (
-                curbuild['commit'], app['Update Check Mode']))
-
-        # Dangerous auto updates
-        if curbuild and app['Auto Update Mode'] != 'None':
-            for flag in ['target', 'srclibs', 'scanignore']:
-                if curbuild[flag]:
-                    pwarn("Auto Update Mode is enabled but '%s' is manually set at '%s'" % (flag, curbuild[flag]))
-
-        # Summary size limit
-        summ_chars = len(app['Summary'])
-        if summ_chars > config['char_limits']['Summary']:
-            warn("Summary of length %s is over the %i char limit" % (
-                summ_chars, config['char_limits']['Summary']))
-
-        # Redundant info
-        if app['Web Site'] and app['Source Code']:
-            if app['Web Site'].lower() == app['Source Code'].lower():
-                warn("Website '%s' is just the app's source code link" % app['Web Site'])
-
-        # "None" still a category
-        if 'None' in app['Categories']:
-            warn("Category 'None' is is still present")
-        elif not app['Categories']:
-            warn("Categories are not set")
-
-        name = app['Name'] or app['Auto Name']
-        if app['Summary'] and name:
-            if app['Summary'].lower() == name.lower():
-                warn("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():
-                warn("Description '%s' is just the app's summary" % app['Summary'])
-
-        # Description size limit
-        desc_charcount = sum(len(l) for l in desc)
-        if desc_charcount > config['char_limits']['Description']:
-            warn("Description of length %s is over the %i char limit" % (
-                desc_charcount, config['char_limits']['Description']))
-
-        if (not desc[0] or not desc[-1]
-                or any(not desc[l - 1] and not desc[l] for l in range(1, len(desc)))):
-            warn("Description has an extra empty line")
-
-        # Regex checks in all kinds of fields
-        for f in regex_warnings:
-            for m, r in regex_warnings[f]:
-                t = metadata.metafieldtype(f)
-                if t == 'string':
-                    if m.match(app[f]):
-                        warn("%s '%s': %s" % (f, app[f], r))
-                elif t == 'multiline':
-                    for l in app[f]:
-                        if m.match(l):
-                            warn("%s at line '%s': %s" % (f, l, r))
-
-        # Build warnings
-        for build in app['builds']:
-            if build['disable']:
-                continue
-            for s in ['master', 'origin', 'HEAD', 'default', 'trunk']:
-                if build['commit'] and build['commit'].startswith(s):
-                    warn("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):
-                        warn("Branch '%s' used as commit in srclib '%s'" % (
-                            s, srclib))
-
-        if not curid:
-            print
-
-    logging.info("Found a total of %i warnings in %i apps out of %i total." % (
-        count['warn'], count['app'], count['app_total']))
+        warns = []
+
+        for check_func in [
+                check_regexes,
+                check_ucm_tags,
+                check_char_limits,
+                check_old_links,
+                check_checkupdates_ran,
+                check_useless_fields,
+                check_empty_fields,
+                check_categories,
+                check_duplicates,
+                check_mediawiki_links,
+                check_bulleted_lists,
+                check_builds,
+                ]:
+            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))
+
+    if anywarns:
+        sys.exit(1)
+
 
 if __name__ == "__main__":
     main()