1.1 release

Fix error message when source tree root directory not found.
Add missing preprocessor support for #ifndef.
Report negative items before neutral items.
Ignore version control blobs.
Fix condition for reporting #error to ignore non-architecture specific macros.
Include the base name of the source tree root directory in the header of the HTML report.
Fix missing icons in the HTML report.

Change-Id: I040ec773579770f6412ef7dd6ed8c9ab6e70b6c1

Author: arm-chrjan01

src/advisor/__init__.py

@@ -17,7 +17,7 @@
 """
 
 __project__ = 'porting-advisor'
-__version__ = '1.0'
+__version__ = '1.1'
 __summary__ = 'Produces an aarch64 porting readiness report.'
 __webpage__ = 'http://www.gitlab.com/arm-hpc/porting-advisor'
 

src/advisor/html_report.py

@@ -18,6 +18,7 @@
 
 from jinja2 import Environment, PackageLoader
 from .report import Report
+import os
 
 
 class HtmlReport(Report):
@@ -30,5 +31,7 @@ def write_items(self, output_file, items):
         )
         template = env.get_template('advice.html')
         rendered = template.render(
-            root_directory=self.root_directory, items=items)
+            root_directory=self.root_directory,
+            root_directory_basename=os.path.basename(self.root_directory),
+            items=items)
         output_file.write(rendered)

src/advisor/main.py

@@ -46,10 +46,10 @@ def main():
     args = parser.parse_args()
 
     if not os.path.exists(args.root):
-        print(_('%s: directory not found.') % args.output, file=sys.stderr)
+        print(_('%s: directory not found.') % args.root, file=sys.stderr)
         sys.exit(1)
     elif not os.path.isdir(args.root):
-        print(_('%s: not a directory.') % args.output, file=sys.stderr)
+        print(_('%s: not a directory.') % args.root, file=sys.stderr)
         sys.exit(1)
 
     if not args.output:

src/advisor/naive_cpp.py

@@ -29,8 +29,18 @@ class PreprocessorDirective:
 
     def __init__(self, directive_type, if_line=None, is_compiler=None):
         self.directive_type = directive_type
+        """
+        The type of directive:
+        
+        TYPE_CONDITIONAL - a #if directive.
+        TYPE_ERROR - a #error directive.
+        TYPE_PRAGMA - a #pragma directive
+        TYPE_OTHER - some other directive.
+        """
         self.if_line = if_line
+        """The line that opened the current preprocessor block."""
         self.is_compiler = is_compiler
+        """True if the current preprocessor block is compiler-speciifc, else False."""
 
 
 class NaiveCpp:
@@ -61,10 +71,31 @@ class NaiveCpp:
 
     def __init__(self):
         self.in_aarch64 = []
+        """
+        Stack of preprocessor block states. When a new preprocessor block is begun with #if or #ifdef a new state
+        is pushed onto the stack. The state is True if the condition contains a macro defined on aarch64, False
+        if the condition contains the negation of a macro defined on aarch64, and None (undefined) otherwise. When the
+        preprocessor block is finished with #endif the state is popped.
+        """
         self.in_other_arch = []
+        """
+        Stack of preprocessor block states. When a new preprocessor block is begun with #if or #ifdef a new state
+        is pushed onto the stack. The state is True if the condition contains a macro defined on a non-aarch64
+        architecture, False if the condition contains the negation of a macro defined on a non-aarch64 architecture, and
+        None (undefined) otherwise. When the preprocessor block is finished with #endif the state is popped.
+        """
         self.in_compiler = []
+        """
+        Stack of preprocessor block states. When a new preprocessor block is begun with #if or #ifdef a new state
+        is pushed onto the stack. The state is True if the condition contains a compiler-specific macro, else False.
+        When the preprocessor block is finished with #endif the state is popped.
+        """
         self.if_lines = []
-        self.in_comment = False
+        """
+        A stack of preprocessor block control statements. When a new preprocessor block is begun with #if or #ifdef
+        the statement is pushed onto the stack. When the preprocessor block is finished with #endif the statement
+        is popped.
+        """
 
     def parse_line(self, line):
         """Parse preprocessor directives in a source line.
@@ -125,6 +156,18 @@ def _parse_directive_line(self, line):
             self.if_lines.append(line)
             return PreprocessorDirective(directive_type=PreprocessorDirective.TYPE_CONDITIONAL, if_line=line,
                                          is_compiler=is_compiler)
+        elif directive == 'ifndef':
+            macro = parts[1]
+            self.in_aarch64.append(
+                False if NaiveCpp.AARCH64_MACROS_RE_PROG.match(macro) is not None else None)
+            self.in_other_arch.append(
+                False if NaiveCpp.NON_AARCH64_MACROS_RE_PROG.match(macro) is not None else None)
+            is_compiler = False if NaiveCpp.COMPILER_MACROS_RE_PROG.match(
+                macro) else None
+            self.in_compiler.append(is_compiler)
+            self.if_lines.append(line)
+            return PreprocessorDirective(directive_type=PreprocessorDirective.TYPE_CONDITIONAL, if_line=line,
+                                         is_compiler=is_compiler)
         elif directive == 'else':
             if self.in_aarch64:
                 self.in_aarch64[-1] = \
@@ -193,29 +236,42 @@ def _is_expression_compiler(expression):
 
     @staticmethod
     def _in_x_code(x):
-        for y in x:
-            if y:
-                return True
-        return False
+        return True in x
+
+    @staticmethod
+    def _in_x_else_code(x):
+        return False in x
 
     def in_aarch64_specific_code(self):
-        """Are we in aarch64 specific code?
+        """
+        Are we in aarch64 specific code?
 
         Returns:
             bool: True if we are currently in an #ifdef __aarch64_ or similar block, else False. 
         """
         return NaiveCpp._in_x_code(self.in_aarch64)
 
     def in_other_arch_specific_code(self):
-        """Are we in other architecture (non-aarch64) specific code?
+        """
+        Are we in other architecture (non-aarch64) specific code?
 
         Returns:
             bool: True if we are currently in an #ifdef OTHERARCH or similar block, else False. 
         """
         return NaiveCpp._in_x_code(self.in_other_arch)
 
+    def in_other_arch_else_code(self):
+        """
+        Are we in the #else block of other architecture (non-aarch64) specific code?
+
+        Returns:
+            bool: True if we are currently in the #else block of an #ifdef OTHERARCH or similar block, else False.
+        """
+        return NaiveCpp._in_x_else_code(self.in_other_arch)
+
     def in_compiler_specific_code(self):
-        """Are we in compiler specific code?
+        """
+        Are we in compiler specific code?
 
         Returns:
             bool: True if we are currently in an #ifdef COMPILER or similar block, else False. 

src/advisor/report_item.py

@@ -26,7 +26,7 @@ class ReportItem:
     NEGATIVE = 'negative'
     ERROR = 'error'
     TYPES = [SUMMARY, POSITIVE,
-             NEUTRAL, NEGATIVE,
+             NEGATIVE, NEUTRAL,
              ERROR]
 
     def __init__(self, description, filename=None, lineno=None, item_type=NEUTRAL):

src/advisor/scanner.py

@@ -23,6 +23,9 @@
 
 
 class Scanner:
+    VCS_SUBDIRECTORIES=['.git', '.hg', '.svn', 'CVS']
+    """List of (hidden) subdirectories used by version control systems."""
+
     def accepts_file(self, filename):
         """Overriden by subclasses to decide whether or not to accept a
         file.
@@ -101,4 +104,6 @@ def scan_tree(self, root, report):
         for dirName, _, fileList in os.walk(root):
             for fname in fileList:
                 path = os.path.join(dirName, fname)
+                if any([('/%s/' % x) in path for x in Scanner.VCS_SUBDIRECTORIES]):
+                    continue
                 self.scan_file(path, report)

src/advisor/source_scanner.py

@@ -80,7 +80,7 @@ def scan_file_object(self, filename, file_object, report):
                 continue
             result = naive_cpp.parse_line(line)
             if result.directive_type == PreprocessorDirective.TYPE_ERROR \
-                    and not naive_cpp.in_other_arch_specific_code():
+                    and naive_cpp.in_other_arch_else_code():
                 preprocessor_errors.append(PreprocessorErrorIssue(filename,
                                                                   lineno,
                                                                   line.strip()))

src/advisor/templates/advice.html

@@ -18,7 +18,7 @@
 -->
 <html>
  <head>
-  <title>Porting Readiness Report for aarch64</title>
+  <title>aarch64 Porting Readiness Report for {{ root_directory_basename }}</title>
   <meta charset="UTF-8">
   <style>
 body {
@@ -69,15 +69,15 @@
    </svg>
   </div>
   <header>
-   <span class="name">Porting Readiness Report for aarch64</span>
+   <span class="name">aarch64 Porting Readiness Report for {{ root_directory_basename }}</span>
   </header>
   <section class="root">
    Source Root: <span class="root_directory">{{ root_directory }}</span>
   </section>
   <section class="advice">
    <table>
     {% for item in items %}
-     <tr><td><svg class="icon"><use xlink:href="#{{ item.type }}"></use></svg></td><td>{{ item.filename if item.filename else '' }}{{ ':' if item.lineno else '' }}{{ item.lineno if item.lineno else '' }}{{ ': ' if item.filename else '' }}{{ item.description }}</td></tr>
+     <tr><td><svg class="icon"><use xlink:href="#{{ item.item_type }}"></use></svg></td><td>{{ item.filename if item.filename else '' }}{{ ':' if item.lineno else '' }}{{ item.lineno if item.lineno else '' }}{{ ': ' if item.filename else '' }}{{ item.description }}</td></tr>
     {% endfor %}
    </table>
   </section>

unittest/test_html_report.py

@@ -0,0 +1,117 @@
+"""
+Copyright 2017 Arm Ltd.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+SPDX-License-Identifier: Apache-2.0
+"""
+
+from advisor.config_guess_scanner import ConfigGuessScanner
+from advisor.html_report import HtmlReport
+from advisor.report_item import ReportItem
+from advisor.source_scanner import SourceScanner
+import io
+import tempfile
+import unittest
+
+
+class TestHtmlReport(unittest.TestCase):
+    def test_item_icons(self):
+        config_guess_scanner = ConfigGuessScanner()
+        source_scanner = SourceScanner()
+
+        report = HtmlReport('/root')
+        io_object = io.StringIO('__asm__("mov r0, r1")')
+        source_scanner.scan_file_object(
+            'test_negative.c', io_object, report)
+        io_object = io.StringIO('#pragma simd foo')
+        source_scanner.scan_file_object(
+            'test_neutral.c', io_object, report)
+        io_object = io.StringIO('aarch64:Linux')
+        config_guess_scanner.scan_file_object(
+            'config.guess', io_object, report)
+        self.assertEquals(len(report.issues), 2)
+        self.assertEquals(len(report.remarks), 1)
+        
+        with tempfile.NamedTemporaryFile(mode='w', delete=False) as ofp:
+            report.write(ofp)
+            fname = ofp.name
+            ofp.close()
+
+            with open(fname) as ifp:
+                for line in ifp:
+                    if 'test_negative.c' in line:
+                        self.assertIn('#negative', line)
+                    elif 'test_neutral.c' in line:
+                        self.assertIn('#neutral', line)
+                    elif 'config.guess' in line:
+                        self.assertIn('#positive', line)
+
+    def test_item_order(self):
+        config_guess_scanner = ConfigGuessScanner()
+        source_scanner = SourceScanner()
+
+        report = HtmlReport('/root')
+        io_object = io.StringIO('__asm__("mov r0, r1")')
+        source_scanner.scan_file_object(
+            'test_negative.c', io_object, report)
+        io_object = io.StringIO('#pragma simd foo')
+        source_scanner.scan_file_object(
+            'test_neutral.c', io_object, report)
+        io_object = io.StringIO('aarch64:Linux')
+        config_guess_scanner.scan_file_object(
+            'config.guess', io_object, report)
+        self.assertEquals(len(report.issues), 2)
+        self.assertEquals(len(report.remarks), 1)
+        
+        with tempfile.NamedTemporaryFile(mode='w', delete=False) as ofp:
+            report.write(ofp)
+            fname = ofp.name
+            ofp.close()
+
+            seenPositive=False
+            seenNeutral=False
+            seenNegative=False
+
+            with open(fname) as ifp:
+                for line in ifp:
+                    if 'test_negative.c' in line:
+                        self.assertTrue(seenPositive)
+                        self.assertFalse(seenNeutral)
+                        seenNegative = True
+                    elif 'test_neutral.c' in line:
+                        self.assertTrue(seenPositive)
+                        self.assertTrue(seenNegative)
+                        seenNeutral = True
+                    elif 'config.guess' in line:
+                        self.assertFalse(seenNeutral)
+                        self.assertFalse(seenNegative)
+                        seenPositive = True
+
+    def test_heading(self):
+        report = HtmlReport('/home/user/source/application-1.0')
+
+        with tempfile.NamedTemporaryFile(mode='w', delete=False) as ofp:
+            report.write(ofp)
+            fname = ofp.name
+            ofp.close()
+
+            seenHeading=False
+
+            with open(fname) as ifp:
+                for line in ifp:
+                    if 'Porting Readiness Report' in line:
+                        self.assertIn('application-1.0', line)
+                        seenHeading = True
+            
+            self.assertTrue(seenHeading)