misra.py: Fix R12.3 false negative in variables declaration (#2453)

Make rule 12.3 check detect commas in the variables declaration code.

addons/misra.py

@@ -1349,15 +1349,83 @@ class MisraChecker:
             if maxval >= sz:
                 self.reportError(token, 12, 2)
 
-    def misra_12_3(self, data):
+    def misra_12_3(self, data, rawTokens, filename):
+        # We need an additional check for closing braces using in
+        # initialization lists and function calls, e.g.:
+        # struct S a = {1, 2, 3}, b, c = foo(1, 2), d;
+        #                       ^                 ^
+        end_tokens_map = {}
+
         for token in data.tokenlist:
-            if token.str != ',' or token.scope.type == 'Enum' or \
+            if token.scope.type in ('Enum', 'Class', 'Struct', 'Global'):
-                    token.scope.type == 'Class' or token.scope.type == 'Global':
                 continue
-            if token.astParent and token.astParent.str in ['(', ',', '{']:
+            # Save end tokens from function calls in initialization
+            if simpleMatch(token, ') ;'):
+                if (token.isExpandedMacro):
+                    end_tokens_map.setdefault(token.next.linenr, set())
+                    end_tokens_map[token.linenr].add(token.next.column)
+                else:
+                    end_tokens_map.setdefault(token.linenr, set())
+                    end_tokens_map[token.linenr].add(token.column)
+            if token.str != ',':
                 continue
+            if token.astParent:
+                if token.astParent.str in ('(', '{'):
+                    end_token = token.astParent.link
+                    if end_token:
+                        end_tokens_map.setdefault(end_token.linenr, set())
+                        end_tokens_map[end_token.linenr].add(end_token.column)
+                    continue
+                elif token.astParent.str == ',':
+                    continue
             self.reportError(token, 12, 3)
 
+        # Cppcheck performs some simplifications in variables declaration code:
+        # int a, b, c;
+        # Will be reresented in dump file as:
+        # int a; int b; int c;
+        name_tokens_map = {}
+        for v in data.variables:
+            if v.isArgument:
+                continue
+            nt = v.nameToken
+            if nt and nt.scope and nt.scope.type not in ('Enum', 'Class', 'Struct'):
+                name_tokens_map.setdefault(nt.linenr, set())
+                name_tokens_map[nt.linenr].add(nt.column)
+        if not name_tokens_map:
+            return
+
+        # Select tokens to check
+        maybe_map = {}
+        for linenr in set(name_tokens_map.keys()) | set(end_tokens_map.keys()):
+            maybe_map[linenr] = name_tokens_map.get(linenr, set())
+            maybe_map[linenr] |= end_tokens_map.get(linenr, set())
+
+        # Check variables declaration code in raw tokens to distinguish ';' and
+        # ',' symbols.
+        STATE_SKIP = 0
+        STATE_CHECK = 1
+        state = STATE_SKIP
+        cur_line = min(maybe_map)
+        for tok in rawTokens:
+            if tok.linenr in maybe_map and tok.column in maybe_map[tok.linenr]:
+                if tok.linenr in end_tokens_map and tok.column in end_tokens_map[tok.linenr]:
+                    if tok.str == ',' or (tok.next and tok.next.str == ','):
+                        self.reportError(tok, 12, 3)
+                        end_tokens_map[tok.linenr].remove(tok.column)
+                state = STATE_CHECK
+                cur_line = tok.linenr
+            if tok.str in ('(', ';', '{'):
+                state = STATE_SKIP
+                if tok.linenr > cur_line:
+                    maybe_map.pop(cur_line)
+                    if not maybe_map:
+                        break
+                    cur_line = min(maybe_map)
+            if state == STATE_CHECK and tok.str == ',':
+                self.reportError(tok, 12, 3)
+
+
     def misra_12_4(self, data):
         if typeBits['INT'] == 16:
             max_uint = 0xffff
@@ -2454,18 +2522,18 @@ class MisraChecker:
         if not self.settings.quiet:
             print(*args, **kwargs)
 
-    def executeCheck(self, rule_num, check_function, arg):
+    def executeCheck(self, rule_num, check_function, *args):
         """Execute check function for a single MISRA rule.
 
         :param rule_num: Number of rule in hundreds format
         :param check_function: Check function to execute
-        :param arg: Check function argument
+        :param args: Check function arguments
         """
         if not self.isRuleGloballySuppressed(rule_num):
-            check_function(arg)
+            check_function(*args)
 
     def parseDump(self, dumpfile):
-
+        filename = '.'.join(dumpfile.split('.')[:-1])
         data = cppcheckdata.parsedump(dumpfile)
 
         self.dumpfileSuppressions = data.suppressions
@@ -2530,7 +2598,7 @@ class MisraChecker:
                 self.executeCheck(1201, self.misra_12_1_sizeof, data.rawTokens)
             self.executeCheck(1201, self.misra_12_1, cfg)
             self.executeCheck(1202, self.misra_12_2, cfg)
-            self.executeCheck(1203, self.misra_12_3, cfg)
+            self.executeCheck(1203, self.misra_12_3, cfg, data.rawTokens, filename)
             self.executeCheck(1204, self.misra_12_4, cfg)
             self.executeCheck(1301, self.misra_13_1, cfg)
             self.executeCheck(1303, self.misra_13_3, cfg)

addons/test/misra/misra-test.c

@@ -332,9 +332,67 @@ void misra_12_2(u8 x) {
   a = x << 8;  // 12.2
 }
 
-void misra_12_3() {
+static int misra_12_3_v1 = 0, misra_12_3_v2; // 12.3
+static int misra_12_3_v3, misra_12_3_v4; // 12.3
+enum misra_12_3_e1 { M123A1, M123B1, M123C1 };
+enum misra_12_3_e2 { M123A2 = 3, M123B2 = 4, M123C2 };
+typedef enum misra_12_3_e3 { M123A3 , M123B3, M123C3 } misra_12_3_e3_t;
+typedef enum { M123A4 , M123B4, M123C4 } misra_12_3_e4_t;
+struct misra_12_3_s1 { int a; int b; int c, d; };
+static struct misra_12_3_s1 misra_12_3_s1_inst = {
+  3,
+  4, 5,
+  6, // no warning
+};
+typedef struct misra_12_3_s2 { int a; int b; int c, d; } misra_12_3_s2_t;
+typedef struct { int a; int b; int c, d; } misra_12_3_s3_t;
+void misra_12_3_fn1(int, int); static int misra_12_3_v5, misra_12_4_v6; // 12.3
+void misra_12_3_fn2(int a, int b) { int d, e; } // 12.3 2.7
+int misra_12_3_fn3(int a, int b) { return a+b;} static int misra_12_3_v5, misra_12_4_v6; // 12.3
+#define MISRA_12_3_FN3_1(A, B) (misra_12_3_fn3(A, B))
+#define MISRA_12_3_FN3_2(A, B) (misra_12_3_fn3(A, \
+                                B))
+#define MISRA_12_3_FN3_2_MSG(x) x, fflush(stderr)
+void misra_12_3(int, int, int); // no warning
+void misra_12_3(int a, int b, int c) { // no warning
+  int a1, a2; // 12.3
+  int a3; int a4; // no warning
+  int a5 = 9, a6; // 12.3
+  int a7, a8 = 11; // 12.3
+  int a9 = foo(), a10; // 12.3
+  int a11 = a = b = c; // 17.8
+
+  struct s1 {int a, b;}; int a12, a13; // 12.3
+  int a14, a15; misra_12_3_fn3(a14, a15); // 12.3 17.7
+  ; int a16, a17; // 12.3
+  int a18; int a19, a20; // 12.3
+  int a21, a22; int a23; // 12.3
+  int a24, // 12.3
+      a25;
+  int a26
+      , a27; // 12.3
+  int a28
+      , // 12.3
+      a29;
+
+  struct misra_12_3_s2 a30 = {1, 2}, a31; // 12.3
+  struct misra_12_3_s2 a32, a33; // 12.3
+  struct misra_12_3_s2 a34, a35 = {1, 2}, a36; // 12.3
+
+  int a37 = MISRA_12_3_FN3_1(a34, a35), a38; // 12.3
+  int a39, a40 = MISRA_12_3_FN3_1(a34, a35); // 12.3
+  int a41 = MISRA_12_3_FN3_2(a34, a35), a42; // 12.3
+  int a43, a44 = MISRA_12_3_FN3_2(a34, a35); // 12.3
+
+  MISRA_12_3_FN3_2_MSG(fprintf(stderr, "test\n")); // 12.3
+
   f((1,2),3); // TODO
-  for (i=0;i<10;i++,j++){} // 12.3
+
+  for (i=0; i<10; i++, j++){} // 12.3
+  for (int i = 0, p = &a1;  // 12.3 14.2
+          i < 42;
+          ++i, ++p ) // 12.3
+  {}
 }
 
 #define MISRA12_4a 2000000000u