# 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
-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://"),
+
+def enforce_https(domain):
+ 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('bitbucket.org'),
+ enforce_https('apache.org'),
+ enforce_https('google.com'),
+ enforce_https('svn.code.sf.net'),
+]
+
+
+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"),
+ (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,
+ 'Issue Tracker': http_checks + [
(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://"),
],
+ 'Donate': http_checks + [
+ (re.compile(r'.*flattr\.com'),
+ "Flattr donation methods belong in the FlattrID flag"),
+ ],
+ 'Changelog': http_checks,
'License': [
(re.compile(r'^(|None|Unknown)$'),
"No license specified"),
'Summary': [
(re.compile(r'^$'),
"Summary yet to be filled"),
+ (re.compile(r'.*\b(free software|open source)\b.*', re.IGNORECASE),
+ "No need to specify that the app is Free Software"),
+ (re.compile(r'.*((your|for).*android|android.*(app|device|client|port|version))', re.IGNORECASE),
+ "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$'),
"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]"),
],
}
-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://.*'),
- "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 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 main():
- global config, options, curid, count
- curid = None
+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'])
+
- count = Counter()
+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 warn(message):
- global curid, count
- if curid:
- print "%s:" % curid
- curid = None
- count['app'] += 1
- print ' %s' % message
- count['warn'] += 1
- def pwarn(message):
- if options.pedantic:
- warn(message)
+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'(^|[^[])\[([^ ]+)( |\]|$)')
+
+
+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)
+
+
+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:
+ 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.method() == 'gradle':
+ yield "target= has no gradle support"
+
+
+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'])
- 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']))
-
- # 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))
-
- # Regex pedantic checks in all kinds of fields
- if options.pedantic:
- for f in regex_pedantic:
- for m, r in regex_pedantic[f]:
- if m.match(app[f]):
- warn("%s '%s': %s" % (f, app[f], 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))
- 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 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()