Ensure that key data can sort

 * add total_ordering and comparison to some models (such as License,
   File, etc)
 * fix some typos in variable names (contributer vs contributor)
 * change order of tag/value for extracted licenses
 * other cosmetics

Signed-off-by: Philippe Ombredanne <[email protected]>

spdx/creationinfo.py

@@ -15,11 +15,13 @@
 from __future__ import unicode_literals
 
 from datetime import datetime
+from functools import total_ordering
 
 from spdx import config
 from spdx import utils
 
 
+@total_ordering
 class Creator(object):
     """
     Creator enity.
@@ -31,9 +33,13 @@ def __init__(self, name):
 
     # FIXME: do not overrride eq and not hash
     def __eq__(self, other):
-        return self.name == other.name
+        return isinstance(other, Creator) and self.name == other.name
 
+    def __lt__(self, other):
+        return isinstance(other, Creator) and self.name < other.name
 
+
+@total_ordering
 class Organization(Creator):
     """
     Organization entity.
@@ -48,13 +54,13 @@ def __init__(self, name, email):
 
     # FIXME: do not overrride eq and not hash
     def __eq__(self, other):
-        if type(other) is not Organization:
-            return False
-        else:
-            return (self.name + self.email) == (other.name + other.email)
+        return isinstance(other, Organization) and (self.name, self.email) == (other.name,  other.email)
+
+    def __lt__(self, other):
+        return isinstance(other, Organization) and (self.name, self.email) < (other.name,  other.email)
 
     def to_value(self):
-        if self.email is not None:
+        if self.email:
             return 'Organization: {0} ({1})'.format(self.name, self.email)
         else:
             return 'Organization: {0}'.format(self.name)
@@ -63,6 +69,7 @@ def __str__(self):
         return self.to_value()
 
 
+@total_ordering
 class Person(Creator):
     """
     Person entity.
@@ -77,10 +84,10 @@ def __init__(self, name, email):
 
     # FIXME: do not overrride eq and not hash
     def __eq__(self, other):
-        if type(other) is not Person:
-            return False
-        else:
-            return (self.name + self.email) == (other.name + other.email)
+        return isinstance(other, Person) and (self.name, self.email) == (other.name,  other.email)
+
+    def __lt__(self, other):
+        return isinstance(other, Person) and (self.name, self.email) < (other.name,  other.email)
 
     def to_value(self):
         if self.email is not None:

spdx/document.py

@@ -14,6 +14,8 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+from functools import total_ordering
+
 from spdx import config
 
 
@@ -25,6 +27,7 @@ def _add_parens(required, text):
     return '({})'.format(text) if required else text
 
 
+@total_ordering
 class License(object):
     def __init__(self, full_name, identifier):
         self._full_name = full_name
@@ -43,9 +46,10 @@ def from_identifier(cls, identifier):
 
     @classmethod
     def from_full_name(cls, full_name):
-        """If the full_name exists in config.LICENSE_MAP
-        the identifier is retrieved from it. Otherwise
-        the identifier is the same as the full_name.
+        """
+        Returna new License for a full_name. If the full_name exists in
+        config.LICENSE_MAP the identifier is retrieved from it.
+        Otherwise the identifier is the same as the full_name.
         """
         if full_name in config.LICENSE_MAP.keys():
             return cls(full_name, config.LICENSE_MAP[full_name])
@@ -69,11 +73,13 @@ def identifier(self):
         return self._identifier
 
     def __eq__(self, other):
-        if isinstance(other, License):
-            return (self.identifier == other.identifier and
-                    self.full_name == other.full_name)
-        else:
-            return False
+        return (
+            isinstance(other, License)
+            and self.identifier == other.identifier
+            and self.full_name == other.full_name)
+
+    def __lt__(self, other):
+        return isinstance(other, License) and self.identifier < other.identifier
 
     def __str__(self):
         return self.identifier
@@ -140,6 +146,7 @@ def identifier(self):
             _add_parens(license_2_complex, self.license_2.identifier))
 
 
+@total_ordering
 class ExtractedLicense(License):
     """
     Represent an ExtractedLicense with its additional attributes:
@@ -154,6 +161,15 @@ def __init__(self, identifier):
         self.cross_ref = []
         self.comment = None
 
+    def __eq__(self, other):
+        return (
+            isinstance(other, ExtractedLicense)
+            and self.identifier == other.identifier
+            and self.full_name == other.full_name)
+
+    def __lt__(self, other):
+        return isinstance(other, ExtractedLicense) and self.identifier < other.identifier
+
     def add_xref(self, ref):
         self.cross_ref.append(ref)
 

spdx/file.py

@@ -14,6 +14,7 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+from functools import total_ordering
 import hashlib
 
 import six
@@ -29,15 +30,15 @@ class FileType(object):
     ARCHIVE = 3
     OTHER = 4
 
-
+@total_ordering
 class File(object):
     """
     Represent an SPDX file.
     Fields:
-    - - name: File name, str mandatory one.
+    - name: File name, str mandatory one.
     - comment: File comment str, Optional zero or one.
     - type: one of FileType.SOURCE, FileType.BINARY, FileType.ARCHIVE
-    - and FileType.OTHER, optional zero or one.
+      and FileType.OTHER, optional zero or one.
     - chk_sum: SHA1, Mandatory one.
     - conc_lics: Mandatory one. document.License or utils.NoAssert or utils.SPDXNone.
     - licenses_in_file: list of licenses found in file, mandatory one or more.
@@ -46,7 +47,7 @@ class File(object):
     - license_comment: Optional.
     - copyright: Copyright text, Mandatory one. utils.NoAssert or utils.SPDXNone or str.
     - notice: optional One, str.
-    - contributers: List of strings.
+    - contributors: List of strings.
     - dependencies: list of file locations.
     - artifact_of_project_name: list of project names, possibly empty.
     - artifact_of_project_home: list of project home page, possibly empty.
@@ -63,30 +64,31 @@ def __init__(self, name, chk_sum=None):
         self.license_comment = None
         self.copyright = None
         self.notice = None
-        self.contributers = []
+        self.contributors = []
         self.dependencies = []
         self.artifact_of_project_name = []
         self.artifact_of_project_home = []
         self.artifact_of_project_uri = []
 
+    def __eq__(self, other):
+        return isinstance(other, File) and self.name == other.name
+
+    def __lt__(self, other):
+        return self.name < other.name
+
     def add_lics(self, lics):
-        """
-        Append lics to licenses_in_file.
-        """
         self.licenses_in_file.append(lics)
 
     def add_contrib(self, contrib):
-        """
-        Append contrib to contributers.
-        """
-        self.contributers.append(contrib)
+        self.contributors.append(contrib)
 
     def add_depend(self, depend):
-        """Appends depend to dependencies."""
         self.dependencies.append(depend)
 
     def add_artifact(self, symbol, value):
-        """Adds value as artifact_of_project{symbol}."""
+        """
+        Add value as artifact_of_project{symbol}.
+        """
         symbol = 'artifact_of_project_{}'.format(symbol)
         artifact = getattr(self, symbol)
         artifact.append(value)

spdx/package.py

@@ -137,7 +137,7 @@ def validate_mandatory_fields(self, messages):
                             'spdx.utils.SPDXNone or spdx.utils.NoAssert or spdx.document.License')
             status = False
 
-        if len(self.licenses_from_files) == 0:
+        if not self.licenses_from_files:
             messages.append('Package licenses_from_files can not be empty')
             status = False
 
@@ -146,7 +146,7 @@ def validate_mandatory_fields(self, messages):
     def validate_files(self, messages):
         # FIXME: we should return messages instead
         messages = messages if messages is not None else []
-        if len(self.files) == 0:
+        if not self.files:
             messages.append('Package must have at least one file.')
             return False
         else: