#492 Code enhancement

Signed-off-by: Chin Yeung Li <[email protected]>

Author: chinyeungli

src/attributecode/gen.py

@@ -70,8 +70,10 @@ def check_duplicated_columns(location):
         dup_msg = u', '.join(dup_msg)
         msg = ('Duplicated column name(s): %(dup_msg)s\n' % locals() +
                'Please correct the input and re-run.')
-        errors.append(Error(ERROR, msg))
-    return unique(errors)
+        err = Error(ERROR, msg)
+        if not err in errors:
+            errors.append(err)
+    return errors
 
 
 def check_duplicated_about_resource(arp, arp_list):
@@ -126,6 +128,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
     """
     errors = []
     abouts = []
+
     if base_dir:
         base_dir = util.to_posix(base_dir)
     if scancode:
@@ -152,16 +155,17 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
             arp = component['about_resource']
             dup_err = check_duplicated_about_resource(arp, arp_list)
             if dup_err:
-                errors.append(dup_err)
+                if not dup_err in errors:
+                    errors.append(dup_err)
             else:
                 arp_list.append(arp)
 
             newline_in_file_err = check_newline_in_file_field(component)
-            for err in newline_in_file_err:
-                errors.append(err)
+            if newline_in_file_err:
+                errors.extend(newline_in_file_err)
 
             invalid_about_filename = check_about_resource_filename(arp)
-            if invalid_about_filename:
+            if invalid_about_filename and not invalid_about_filename in errors:
                 errors.append(invalid_about_filename)
         if errors:
             return errors, abouts
@@ -172,7 +176,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
         errors.append(Error(CRITICAL, msg))
         return errors, abouts
 
-    for i, fields in enumerate(inventory):
+    for fields in inventory:
         # check does the input contains the required fields
         required_fields = model.About.required_fields
 
@@ -223,9 +227,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
             if e.message == 'Field about_resource is required':
                 ld_errors.remove(e)
         """
-        for e in ld_errors:
-            if not e in errors:
-                errors.extend(ld_errors)
+        errors.extend(ld_errors)
         abouts.append(about)
     # Covert the license_score value from string to list of int
     # The licesne_score is not in the spec but is specify in the scancode license scan.
@@ -239,7 +241,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
             except:
                 pass
 
-    return unique(errors), abouts
+    return errors, abouts
 
 
 def update_about_resource(self):
@@ -285,6 +287,8 @@ def generate(location, base_dir, android=None, reference_dir=None, fetch_license
                     errors.append(e)
 
     for about in abouts:
+        # Strip trailing spaces
+        about.about_file_path = about.about_file_path.strip()
         if about.about_file_path.startswith('/'):
             about.about_file_path = about.about_file_path.lstrip('/')
         dump_loc = join(bdir, about.about_file_path.lstrip('/'))
@@ -328,14 +332,12 @@ def generate(location, base_dir, android=None, reference_dir=None, fetch_license
                     msg = (u'Field about_resource: '
                            u'%(path)s '
                            u'does not exist' % locals())
-                    not_exist_errors.append(msg)
+                errors.append(Error(INFO, msg))
 
             licenses_dict = {}
             if gen_license:
                 # Write generated LICENSE file
-                license_key_name_context_url_list, err = about.dump_lic(dump_loc, license_dict)
-                if err:
-                    errors.append(err)
+                license_key_name_context_url_list = about.dump_lic(dump_loc, license_dict)
                 if license_key_name_context_url_list:
                     for lic_key, lic_name, lic_filename, lic_context, lic_url, spdx_lic_key in license_key_name_context_url_list:
                         licenses_dict[lic_key] = [lic_name, lic_filename, lic_context, lic_url, spdx_lic_key]
@@ -372,9 +374,6 @@ def generate(location, base_dir, android=None, reference_dir=None, fetch_license
                 else:
                     notice_dict[notice_path] = notice_context
 
-            for e in not_exist_errors:
-                errors.append(Error(INFO, e))
-
         except Exception as e:
             # only keep the first 100 char of the exception
             # TODO: truncated errors are likely making diagnotics harder
@@ -393,4 +392,4 @@ def generate(location, base_dir, android=None, reference_dir=None, fetch_license
             else:
                 about.dump_android_notice(path, notice_dict[path])
 
-    return unique(errors), abouts
+    return errors, abouts

src/attributecode/model.py

@@ -879,6 +879,7 @@ def hydrate(self, fields):
         """
         errors = []
         seen_fields = {}
+        illegal_name_list = []
 
         for name, value in fields:
             orig_name = name
@@ -916,10 +917,16 @@ def hydrate(self, fields):
 
             # A custom field
             # is the name valid?
+            if not is_valid_name(name):
+                if not name in illegal_name_list:
+                    illegal_name_list.append(name)
+                continue
+            """
             illegal_name_error = validate_field_name(name)
             if illegal_name_error:
-                errors.append(illegal_name_error)
+                errors.append(illegal_name_list)
                 continue
+            """
 
             msg = 'Field %(orig_name)s is a custom field.'
             errors.append(Error(INFO, msg % locals()))
@@ -945,6 +952,10 @@ def hydrate(self, fields):
                     msg = 'Internal error with custom field: %(name)r: %(value)r.'
                     errors.append(Error(CRITICAL, msg % locals()))
 
+        if illegal_name_list:
+            msg = ('Field name: %(illegal_name_list)r contains illegal name characters: '
+               '0 to 9, a to z, A to Z and _. (or empty spaces) and is ignored.')
+            errors.append(Error(CRITICAL, msg % locals()))
         return errors
 
     def process(self, fields, about_file_path, running_inventory=False,
@@ -1034,7 +1045,7 @@ def load_dict(self, fields_dict, base_dir, scancode=False, from_attrib=False, ru
 
         for key, value in fields:
             if not value:
-                # never return empty or absent fieds
+                # never return empty or absent fields
                 continue
             if key == u'licenses':
                 # FIXME: use a license object instead
@@ -1147,11 +1158,14 @@ def dumps(self, licenses_dict=None):
             if licenses_dict and lic_key in licenses_dict:
                 lic_dict['key'] = lic_key
                 lic_name, lic_filename, lic_context, lic_url, spdx_lic_key = licenses_dict[lic_key]
-
-                lic_dict['name'] = lic_name
-                lic_dict['file'] = lic_filename
-                lic_dict['url'] = lic_url
-                lic_dict['spdx_license_key'] = spdx_lic_key
+                if lic_name:
+                    lic_dict['name'] = lic_name
+                if lic_filename:
+                    lic_dict['file'] = lic_filename
+                if lic_url:
+                    lic_dict['url'] = lic_url
+                if spdx_lic_key:
+                    lic_dict['spdx_license_key'] = spdx_lic_key
 
                 # Remove the license information if it has been handled
                 lic_key_copy.remove(lic_key)
@@ -1161,6 +1175,8 @@ def dumps(self, licenses_dict=None):
                     license_url.remove(lic_url)
                 if lic_filename in license_file:
                     license_file.remove(lic_filename)
+                if spdx_lic_key in spdx_license_key:
+                    spdx_license_key.remove(spdx_lic_key)
                 lic_dict_list.append(lic_dict)
 
         # Handle license information that have not been handled.
@@ -1277,7 +1293,6 @@ def dump_lic(self, location, license_dict):
         loc = util.to_posix(location)
         parent = posixpath.dirname(loc)
         license_key_name_context_url = []
-        err = ''
 
         if not posixpath.exists(parent):
             os.makedirs(add_unc(parent))
@@ -1288,20 +1303,26 @@ def dump_lic(self, location, license_dict):
             self.license_key.present = True
             if not special_char_in_expression:
                 for lic_key in lic_list:
-                    try:
-                        if license_dict[lic_key]:
-                            license_path = posixpath.join(parent, lic_key)
-                            license_path += u'.LICENSE'
-                            license_path = add_unc(license_path)
-                            license_name, license_filename, license_context, license_url, spdx_license_key = license_dict[lic_key]
-                            license_info = (lic_key, license_name, license_filename, license_context, license_url, spdx_license_key)
-                            license_key_name_context_url.append(license_info)
-                            with io.open(license_path, mode='w', encoding='utf-8', newline='\n', errors='replace') as lic:
-                                lic.write(license_context)
-                    except Exception as e:
-                        err = Error(ERROR, 'Invalid license: ' + str(e))
-
-        return license_key_name_context_url, err
+                    license_name = ''
+                    license_filename = ''
+                    license_context = ''
+                    license_url = ''
+                    spdx_license_key = ''
+                    if lic_key in license_dict:
+                        license_path = posixpath.join(parent, lic_key)
+                        license_path += u'.LICENSE'
+                        license_path = add_unc(license_path)
+                        license_name, license_filename, license_context, license_url, spdx_license_key = license_dict[lic_key]
+                        license_info = (lic_key, license_name, license_filename, license_context, license_url, spdx_license_key)
+                        license_key_name_context_url.append(license_info)
+                        with io.open(license_path, mode='w', encoding='utf-8', newline='\n', errors='replace') as lic:
+                            lic.write(license_context)
+                    else:
+                        # Invalid license issue is already handled
+                        license_info = (lic_key, license_name, license_filename, license_context, license_url, spdx_license_key)
+                        license_key_name_context_url.append(license_info)
+
+        return license_key_name_context_url
 
 
 def collect_inventory(location):
@@ -1324,7 +1345,7 @@ def collect_inventory(location):
             msg = (about_file_path + ": " + message)
             errors.append(Error(severity, msg))
         abouts.append(about)
-    return unique(errors), abouts
+    return errors, abouts
 
 
 def collect_abouts_license_expression(location):