chiark / gitweb /
Apply some autopep8-python2 suggestions
[fdroidserver.git] / fdroidserver / lint.py
index bd7a43ccd4ce5d594ea678f2a1a2f2579302856c..8cf1bf61d903d340f44c8f121f7ae1f8dbbd71d5 100644 (file)
 from optparse import OptionParser
 import re
 import logging
-import common, metadata
+import common
+import metadata
+from collections import Counter
 
 config = None
 options = None
 
 regex_warnings = {
-        'Web Site': [
-            (re.compile(r'.*[^sS]://github\.com/.*'),
-                "github URLs should always use https:// not http://"),
-            (re.compile(r'.*[^sS]://code\.google\.com/.*'),
-                "code.google.com URLs should always use https:// not http://"),
-        ],
-        'Source Code': [
-            (re.compile(r'.*[^sS]://github\.com/.*'),
-                "github URLs should always use https:// (not http://, git://, or git@)"),
-            (re.compile(r'.*code\.google\.com/p/[^/]+[/]*$'),
-                "/source is missing"),
-            (re.compile(r'.*[^sS]://code\.google\.com/.*'),
-                "code.google.com URLs should always use https:// not http://"),
-            (re.compile(r'.*[^sS]://dl\.google\.com/.*'),
-                "dl.google.com URLs should always use https:// not http://"),
-            (re.compile(r'.*[^sS]://gitorious\.org/.*'),
-                "gitorious URLs should always use https:// (not http://, git://, or git@)"),
-        ],
-        'Repo': [
-            (re.compile(r'.*[^sS]://code\.google\.com/.*'),
-                "code.google.com URLs should always use https:// not http://"),
-            (re.compile(r'.*[^sS]://dl\.google\.com/.*'),
-                "dl.google.com URLs should always use https:// not http://"),
-            (re.compile(r'.*[^sS]://github\.com/.*'),
-                "github URLs should always use https:// (not http://, git://, or git@)"),
-            (re.compile(r'.*[^sS]://gitorious\.org/.*'),
-                "gitorious URLs should always use https:// (not http://, git://, or git@)"),
-            (re.compile(r'.*[^sS]://[^.]*\.googlecode\.com/svn/?.*'),
-                "Google Code SVN URLs should always use https:// (not http:// or svn://)"),
-            (re.compile(r'.*[^sS]://svn\.apache\.org/repos/?.*'),
-                "Apache SVN URLs should always use https:// (not http:// or svn://)"),
-            (re.compile(r'.*[^sS]://svn\.code\.sf\.net/.*'),
-                "Sourceforge SVN URLs should always use https:// (not http:// or svn://)"),
-        ],
-        'Issue Tracker': [
-            (re.compile(r'.*code\.google\.com/p/[^/]+[/]*$'),
-                "/issues is missing"),
-            (re.compile(r'.*[^sS]://code\.google\.com/.*'),
-                "code.google.com URLs should always use https:// not http://"),
-            (re.compile(r'.*github\.com/[^/]+/[^/]+[/]*$'),
-                "/issues is missing"),
-            (re.compile(r'.*[^sS]://github\.com/.*'),
-                "github URLs should always use https:// not http://"),
-            (re.compile(r'.*[^sS]://gitorious\.org/.*'),
-                "gitorious URLs should always use https:// not http://"),
-        ],
+    'Web Site': [
+        (re.compile(r'.*[^sS]://github\.com/.*'),
+         "github URLs should always use https:// not http://"),
+        (re.compile(r'.*[^sS]://code\.google\.com/.*'),
+         "code.google.com URLs should always use https:// not http://"),
+    ],
+    'Source Code': [
+        (re.compile(r'.*[^sS]://github\.com/.*'),
+         "github URLs should always use https:// (not http://, git://, or git@)"),
+        (re.compile(r'.*code\.google\.com/p/[^/]+[/]*$'),
+         "/source is missing"),
+        (re.compile(r'.*[^sS]://code\.google\.com/.*'),
+         "code.google.com URLs should always use https:// not http://"),
+        (re.compile(r'.*[^sS]://dl\.google\.com/.*'),
+         "dl.google.com URLs should always use https:// not http://"),
+        (re.compile(r'.*[^sS]://gitorious\.org/.*'),
+         "gitorious URLs should always use https:// (not http://, git://, or git@)"),
+    ],
+    'Repo': [
+        (re.compile(r'.*[^sS]://code\.google\.com/.*'),
+         "code.google.com URLs should always use https:// not http://"),
+        (re.compile(r'.*[^sS]://dl\.google\.com/.*'),
+         "dl.google.com URLs should always use https:// not http://"),
+        (re.compile(r'.*[^sS]://github\.com/.*'),
+         "github URLs should always use https:// (not http://, git://, or git@)"),
+        (re.compile(r'.*[^sS]://gitorious\.org/.*'),
+         "gitorious URLs should always use https:// (not http://, git://, or git@)"),
+        (re.compile(r'.*[^sS]://[^.]*\.googlecode\.com/svn/?.*'),
+         "Google Code SVN URLs should always use https:// (not http:// or svn://)"),
+        (re.compile(r'.*[^sS]://svn\.apache\.org/repos/?.*'),
+         "Apache SVN URLs should always use https:// (not http:// or svn://)"),
+        (re.compile(r'.*[^sS]://svn\.code\.sf\.net/.*'),
+         "Sourceforge SVN URLs should always use https:// (not http:// or svn://)"),
+    ],
+    'Issue Tracker': [
+        (re.compile(r'.*code\.google\.com/p/[^/]+[/]*$'),
+         "/issues is missing"),
+        (re.compile(r'.*[^sS]://code\.google\.com/.*'),
+         "code.google.com URLs should always use https:// not http://"),
+        (re.compile(r'.*github\.com/[^/]+/[^/]+[/]*$'),
+         "/issues is missing"),
+        (re.compile(r'.*[^sS]://github\.com/.*'),
+         "github URLs should always use https:// not http://"),
+        (re.compile(r'.*[^sS]://gitorious\.org/.*'),
+         "gitorious URLs should always use https:// not http://"),
+    ],
+    'License': [
+        (re.compile(r'^(|None|Unknown)$'),
+         "No license specified"),
+    ],
+    'Summary': [
+        (re.compile(r'^$'),
+         "Summary yet to be filled"),
+    ],
+    'Description': [
+        (re.compile(r'^No description available$'),
+         "Description yet to be filled"),
+        (re.compile(r'\s*[*#][^ .]'),
+         "Invalid bulleted list"),
+        (re.compile(r'^\s'),
+         "Unnecessary leading space"),
+        (re.compile(r'.*\s$'),
+         "Unnecessary trailing space"),
+    ],
 }
 
 regex_pedantic = {
-        'Web Site': [
-            (re.compile(r'.*github\.com/[^/]+/[^/]+\.git'),
-                "Appending .git is not necessary"),
-            (re.compile(r'.*code\.google\.com/p/[^/]+/[^w]'),
-                "Possible incorrect path appended to google code project site"),
-        ],
-        'Source Code': [
-            (re.compile(r'.*github\.com/[^/]+/[^/]+\.git'),
-                "Appending .git is not necessary"),
-            (re.compile(r'.*code\.google\.com/p/[^/]+/source/.*'),
-                "/source is often enough on its own"),
-        ],
-        'Repo': [
-            (re.compile(r'^http://.*'),
-                "if https:// is available, use it instead of http://"),
-            (re.compile(r'^svn://.*'),
-                "if https:// is available, use it instead of svn://"),
-        ],
-        'Issue Tracker': [
-            (re.compile(r'.*code\.google\.com/p/[^/]+/issues/.*'),
-                "/issues is often enough on its own"),
-            (re.compile(r'.*github\.com/[^/]+/[^/]+/issues/.*'),
-                "/issues is often enough on its own"),
-        ],
-        'Summary': [
-            (re.compile(r'.*\bandroid\b.*', re.IGNORECASE),
-                "No need to specify that the app is for Android"),
-            (re.compile(r'.*\b(app|application)\b.*', re.IGNORECASE),
-                "No need to specify that the app is... an app"),
-            (re.compile(r'.*\b(free software|open source)\b.*', re.IGNORECASE),
-                "No need to specify that the app is Free Software"),
-            (re.compile(r'.*[a-z0-9][.,!?][ $]'),
-                "Punctuation should be avoided"),
-        ],
+    'Web Site': [
+        (re.compile(r'.*github\.com/[^/]+/[^/]+\.git'),
+         "Appending .git is not necessary"),
+        (re.compile(r'.*code\.google\.com/p/[^/]+/[^w]'),
+         "Possible incorrect path appended to google code project site"),
+    ],
+    'Source Code': [
+        (re.compile(r'.*github\.com/[^/]+/[^/]+\.git'),
+         "Appending .git is not necessary"),
+        (re.compile(r'.*code\.google\.com/p/[^/]+/source/.*'),
+         "/source is often enough on its own"),
+    ],
+    'Repo': [
+        (re.compile(r'^http://.*'),
+         "use https:// if available"),
+        (re.compile(r'^svn://.*'),
+         "use https:// if available"),
+    ],
+    'Issue Tracker': [
+        (re.compile(r'.*code\.google\.com/p/[^/]+/issues/.*'),
+         "/issues is often enough on its own"),
+        (re.compile(r'.*github\.com/[^/]+/[^/]+/issues/.*'),
+         "/issues is often enough on its own"),
+    ],
+    'Summary': [
+        (re.compile(r'.*\b(free software|open source)\b.*', re.IGNORECASE),
+         "No need to specify that the app is Free Software"),
+        (re.compile(r'.*[a-z0-9][.,!?][ $]'),
+         "Punctuation should be avoided"),
+    ],
 }
 
+
 def main():
 
-    global config, options, appid, app_count, warn_count
-    appid = None
+    global config, options, curid, count
+    curid = None
 
-    app_count = 0
-    warn_count = 0
+    count = Counter()
 
     def warn(message):
-        global appid, app_count, warn_count
-        if appid:
-            print "%s:" % appid
-            appid = None
-            app_count += 1
+        global curid, count
+        if curid:
+            print "%s:" % curid
+            curid = None
+            count['app'] += 1
         print '    %s' % message
-        warn_count += 1
+        count['warn'] += 1
 
     def pwarn(message):
         if options.pedantic:
@@ -134,10 +150,12 @@ def main():
 
     # Parse command line...
     parser = OptionParser(usage="Usage: %prog [options] [APPID [APPID ...]]")
-    parser.add_option("-p", "--pedantic", action="store_true", default=False,
-                      help="Show pedantic warnings that might give false positives")
     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()
 
     config = common.read_config(options)
@@ -146,26 +164,30 @@ def main():
     allapps = metadata.read_metadata(xref=False)
     apps = common.read_app_args(args, allapps, False)
 
-    for app in apps:
-        appid = app['id']
-        lastcommit = ''
-
+    for appid, app in apps.iteritems():
         if app['Disabled']:
             continue
 
+        curid = appid
+        count['app_total'] += 1
+
+        curbuild = None
         for build in app['builds']:
-            if 'commit' in build and 'disable' not in build:
-                lastcommit = build['commit']
+            if not curbuild or int(build['vercode']) > int(curbuild['vercode']):
+                curbuild = build
 
         # Potentially incorrect UCM
-        if (app['Update Check Mode'] == 'RepoManifest' and
-                any(s in lastcommit for s in '.,_-/')):
+        if (curbuild and curbuild['commit']
+                and app['Update Check Mode'] == 'RepoManifest' and
+                any(s in curbuild['commit'] for s in '.,_-/')):
             pwarn("Last used commit '%s' looks like a tag, but Update Check Mode is '%s'" % (
-                lastcommit, app['Update Check Mode']))
+                curbuild['commit'], app['Update Check Mode']))
 
-        # No proper license
-        if app['License'] in ('Unknown','None',''):
-            warn("License was not set")
+        # 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'])
@@ -173,28 +195,23 @@ def main():
             warn("Summary of length %s is over the %i char limit" % (
                 summ_chars, config['char_limits']['Summary']))
 
-        # Redundant summaries
-        summary = app['Summary']
-        name = str(app['Name'] if app['Name'] else app['Auto Name'])
-        if summary and name:
-            summary_l = summary.lower()
-            name_l = name.lower()
-            if summary_l == name_l:
-                warn("Summary '%s' is just the app's name" % summary)
-            elif (summary_l in name_l or name_l in summary_l):
-                pwarn("Summary '%s' probably contains redundant info already in app name '%s'" % (
-                    summary, name))
-
-        # Invalid lists
-        desc_chars = 0
-        for line in app['Description']:
-            if re.match(r'[ ]*[*#][^ .]', line):
-                warn("Invalid bulleted list: '%s'" % line)
-            if re.match(r'^ ', line):
-                warn("Unnecessary leading space: '%s'" % line)
-            desc_chars += len(line)
+        # 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'])
+                app['Web Site'] = ''
+
+        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'])
+
+        if app['Summary'] and app['Description'] and len(app['Description']) == 1:
+            if app['Summary'].lower() == app['Description'][0].lower():
+                warn("Description '%s' is just the app's summary" % app['Summary'])
 
         # Description size limit
+        desc_chars = sum(len(l) for l in app['Description'])
         if desc_chars > config['char_limits']['Description']:
             warn("Description of length %s is over the %i char limit" % (
                 desc_chars, config['char_limits']['Description']))
@@ -202,8 +219,14 @@ def main():
         # Regex checks in all kinds of fields
         for f in regex_warnings:
             for m, r in regex_warnings[f]:
-                if m.match(app[f]):
-                    warn("%s '%s': %s" % (f, app[f], r))
+                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))
 
         # Regex pedantic checks in all kinds of fields
         if options.pedantic:
@@ -214,23 +237,30 @@ def main():
 
         # Build warnings
         for build in app['builds']:
-            for n in ['master', 'origin/', 'default', 'trunk']:
-                if 'commit' in build:
-                    if build['commit'].startswith(n):
-                        warn("Branch '%s' used as commit in build '%s'" % (
-                            n, build['version']))
-                if 'srclibs' in build:
-                    for srclib in build['srclibs']:
-                        ref = srclib.split('@')[1].split('/')[0]
-                        if ref.startswith(n):
-                            warn("Branch '%s' used as commit in srclib '%s'" % (
-                                n, srclib))
-
-        if not appid:
+            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))
+            for s in ['git clone', 'git svn clone', 'svn checkout', 'svn co', 'hg clone']:
+                for flag in ['init', 'prebuild', 'build']:
+                    if not build[flag]:
+                        continue
+                    if s in build[flag]:
+                        # TODO: This should not be pedantic!
+                        pwarn("'%s' used in %s '%s'" % (s, flag, build[flag]))
+
+        if not curid:
             print
 
-    logging.info("Found a total of %i warnings in %i apps." % (warn_count, app_count))
+    logging.info("Found a total of %i warnings in %i apps out of %i total." % (
+        count['warn'], count['app'], count['app_total']))
 
 if __name__ == "__main__":
     main()
-