Rewrote find_project_root_and_manifest from scratch.

ide/tests/test_find_project_root.py

@@ -99,7 +99,7 @@ def test_ignore_invalid_package_file(self):
         """ If a project has an invalid package.json and an appinfo.json, select the latter """
         self.run_test([
             FakeProjectItem("package.json", "{}"),
-            "src/"
+            "src/",
             "src/main.c",
             "appinfo.json"
         ], "", "appinfo.json")
@@ -109,7 +109,7 @@ def test_PR_317(self):
         self.run_test([
             "MAINTAINERS",
             "package.json",
-            "src/"
+            "src/",
             "src/main.c",
         ], "", "package.json")
 
@@ -121,3 +121,9 @@ def test_find_most_shallow_project(self):
             "package.json",
             "src/main.c",
         ], "", "package.json")
+
+
+    def test_malformed_names_bug(self):
+        """ Test for a bug where characters could be prepended to manifest names. """
+        with self.assertRaises(InvalidProjectArchiveException):
+            self.run_test(["project/rrrpackage.json", "project/rrrsrc/", "project/rrrsrc/main.c"])

ide/utils/project.py

@@ -1,12 +1,12 @@
 import abc
 import json
-import os.path
+import os
 
 from django.utils.translation import ugettext as _
 
 __author__ = 'katharine'
 
-
+SRC_DIR = 'src'
 PACKAGE_MANIFEST = 'package.json'
 APPINFO_MANIFEST = 'appinfo.json'
 MANIFEST_KINDS = [PACKAGE_MANIFEST, APPINFO_MANIFEST]
@@ -33,57 +33,39 @@ def path(self):
         return None
 
 
-def is_manifest(kind, contents):
-    """ A potentially valid manifest is a package.json file with a "pebble" object, or an appinfo.json file. """
-    if kind == PACKAGE_MANIFEST:
-        return 'pebble' in json.loads(contents)
-    elif kind == APPINFO_MANIFEST:
-        return True
-    else:
-        return False
+def rank_manifest_path(dirname, kind):
+    """ Sort key for manifest files. Sort first by depth and add a penalty for being an appinfo.json """
+    return os.path.normpath(dirname).count('/') + (0.5 if kind == APPINFO_MANIFEST else 0)
 
 
 def find_project_root_and_manifest(project_items):
     """ Given the contents of an archive, find a valid Pebble project.
     :param project_items: A list of BaseProjectItems
     :return: A tuple of (path_to_project, manifest BaseProjectItem)
     """
-    SRC_DIR = 'src/'
-
-    # Sort the paths by the number of path separators they have,
-    sorted_items = sorted(project_items, key=lambda x: os.path.normpath(x.path).count('/'))
-    for i, item in enumerate(sorted_items):
-        base_dir = item.path
-
-        # Check if the file is one of the kinds of manifest file
-        for name in MANIFEST_KINDS:
-            dir_end = base_dir.rfind(name)
-            if dir_end == -1:
-                continue
-            # Ensure that the file is actually a manifest file
-            if dir_end + len(name) == len(base_dir):
-                if is_manifest(name, item.read()):
-                    manifest_item = item
-                    break
-        else:
-            # If the file is not a manifest file, continue looking for the manfiest.
-            continue
+    found_manifests = set()
+    found_src_directories = set()
 
-        # The base dir is the location of the manifest file without the manifest filename.
-        base_dir = base_dir[:dir_end]
-
-        # Now check the rest of the items for a source directory containing at least one source file.
-        for source_item in sorted_items[i+1:]:
-            source_dir = source_item.path
-            if source_dir[:dir_end] != base_dir:
-                continue
-            if not source_dir.endswith('.c') and not source_dir.endswith('.js'):
-                continue
-            if source_dir[dir_end:dir_end + len(SRC_DIR)] != SRC_DIR:
-                continue
-            break
-        else:
-            # If there was no source directory with a source file, keep looking for manifest files.
+    for item in project_items:
+        item_path = item.path
+
+        # If the item looks like a manifest, add it to a set of potential manifests
+        item_dirname, item_basename = os.path.split(item_path)
+        if (item_basename == PACKAGE_MANIFEST and 'pebble' in json.loads(item.read())) or item_basename == APPINFO_MANIFEST:
+            found_manifests.add(((item_dirname, item_basename), item))
             continue
-        return base_dir, manifest_item
+
+        # Otherwise, check if the file is a source file
+        if item_path.endswith(('.c', '.js')):
+            # If it is a source file in an 'src' directory, add its parent to the set of potential project directories
+            source_dir_dirname, source_dir_basename = os.path.split(item_dirname)
+            if source_dir_basename == SRC_DIR:
+                found_src_directories.add(source_dir_dirname)
+
+    # Choose the most shallow manifest file which has a non-empty source directory.
+    sorted_manifests = sorted(found_manifests, key=lambda x: rank_manifest_path(*x[0]))
+    for (base, kind), item in sorted_manifests:
+        if base in found_src_directories:
+            base = base + os.sep if base else ""
+            return base + os.sep, item
     raise InvalidProjectArchiveException(_("No project root found."))