Initial work to use 'cpplint' to scan C sources. (#50)

.github/scripts/run_pylint_check.py

@@ -1,15 +1,18 @@
 # pylint: skip-file
 
 import argparse
+import glob
 import io
 import os
+import subprocess
 import sys
 
 from github import Github
 from pylint import lint as pl_run
 from pylint.__pkginfo__ import __version__ as pl_version
 from cython_lint import cython_lint as cl_run
 from cython_lint import __version__ as cl_version
+from cpplint import __VERSION__ as cp_version
 
 
 def main():
@@ -18,34 +21,33 @@ def main():
         "Check if any opened issues have been closed, run linters, and open an issue if any complain")
     parser.add_argument("--token", help="The GitHub API token to use")
     parser.add_argument("--repo", help="The owner and repository we are operating on")
-    parser.add_argument("--label-pylint", help="The name of the GitHub label for 'pylint' generated issues")
-    parser.add_argument("--label-cython", help="The name of the GitHub label for 'cython-lint' generated issues")
     args = parser.parse_args()
     # Connect to GitHub REST API
     gh = Github(args.token)
     # Run the linters
-    pylint(gh, args.repo, args.label_pylint)
-    cython_lint(gh, args.repo, args.label_cython)
+    pylint(gh, args.repo)
+    cython_lint(gh, args.repo)
+    cpplint(gh, args.repo)
 
 
-def _check_issues(gh, repo, tool, label):
-    open_issues = gh.search_issues(f"repo:{repo} label:{label} is:issue is:open")
+def _check_issues(gh, repo, tool):
+    open_issues = gh.search_issues(f"repo:{repo} label:automated/{tool} is:issue is:open")
     if open_issues.totalCount != 0:
         print(f"Skipping '{tool}' run due to existing issue {open_issues[0].html_url}.")
         return True
     else:
         return False
 
 
-def _file_issue(gh, repo, tool, tool_args, tool_version, label, output):
+def _file_issue(gh, repo, tool, tool_args, tool_version, output):
     issue_title = f"New version of pylint ({tool_version}) identifies new issues"
     issue_body = (f"A version of `{tool}` is available in the Python package repositories that identifies issues "
                   f"with the `spotfire` package.  Since we attempt to keep all lint issues out of the source "
                   f"code (either by fixing the issue identified or by disabling that message with a localized "
                   f"comment), this is indicative of a new check in this new version of `{tool}`.\n\n"
                   f"Please investigate these issues, and either fix the source or disable the check with a "
                   f"comment.  Further checks by this automation will be held until this issue is closed.  Make "
-                  f"sure that the fix updates the `{tool}` requirement in `requirements_lint.txt` to the version "
+                  f"sure that the fix updates the `{tool}` requirement in `pyproject.toml` to the version "
                   f"identified here ({tool_version}).\n\n"
                   f"For reference, here is the output of this version of `{tool}`:\n\n"
                   f"```\n"
@@ -54,7 +56,7 @@ def _file_issue(gh, repo, tool, tool_args, tool_version, label, output):
                   f"```\n\n"
                   f"*This issue was automatically opened by the `pylint.yaml` workflow.*\n")
     repo = gh.get_repo(repo)
-    repo_label = repo.get_label(label)
+    repo_label = repo.get_label(f"automated/{tool}")
     new_issue = repo.create_issue(title=issue_title, body=issue_body, labels=[repo_label])
     print(f"Opened issue {new_issue.html_url}")
 
@@ -76,9 +78,9 @@ def output(self):
         return self._capture.getvalue()
 
 
-def pylint(gh, repo, label):
+def pylint(gh, repo):
     # Determine if we should run pylint
-    if _check_issues(gh, repo, "pylint", label):
+    if _check_issues(gh, repo, "pylint"):
         return
 
     # Now run pylint
@@ -88,12 +90,12 @@ def pylint(gh, repo, label):
         return
 
     # File an issue
-    _file_issue(gh, repo, "pylint", "spotfire", pl_version, label, capture.output())
+    _file_issue(gh, repo, "pylint", "spotfire", pl_version, capture.output())
 
 
-def cython_lint(gh, repo, label):
+def cython_lint(gh, repo):
     # Determine if we should run cython-lint
-    if _check_issues(gh, repo, "cython-lint", label):
+    if _check_issues(gh, repo, "cython-lint"):
         return
 
     # Now run cython-lint
@@ -103,7 +105,23 @@ def cython_lint(gh, repo, label):
         return
 
     # File an issue
-    _file_issue(gh, repo, "cython-lint", "spotfire vendor", cl_version, label, capture.output())
+    _file_issue(gh, repo, "cython-lint", "spotfire vendor", cl_version, capture.output())
+
+
+def cpplint(gh, repo):
+    # Determine if we should run cpplint
+    if _check_issues(gh, repo, "cpplint"):
+        return
+
+    # Now run cpplint
+    command = [sys.executable, "-m", "cpplint"]
+    command.extend(glob.glob("spotfire/*_helpers.[ch]"))
+    result = subprocess.run(command, capture_output=True, check=False)
+    if not result.returncode:
+        return
+
+    # File an issue
+    _file_issue(gh, repo, "cpplint", "spotfire/*_helpers.[ch]", cp_version, result.stdout.decode("utf-8"))
 
 
 if __name__ == "__main__":

.github/workflows/build.yaml

@@ -37,6 +37,7 @@ jobs:
             **/*.pyx
             **/*.pxd
             **/*.pxi
+            spotfire/*_helpers.*
       - name: Dynamic Elements
         id: dynamic
         run: |
@@ -167,3 +168,4 @@ jobs:
           pylint spotfire
           mv .save/* . && rmdir .save
           cython-lint spotfire vendor
+          find spotfire -name '*_helpers.[ch]' | xargs cpplint

.github/workflows/pylint.yaml

@@ -14,7 +14,7 @@ jobs:
           python-version: '3.x'
       - name: Install Tools
         run: |
-          pip install PyGithub pylint cython-lint
+          pip install PyGithub pylint cython-lint cpplint
       - name: Run Analysis Tools
         run: |
-          python .github/scripts/run_pylint_check.py --token ${{ secrets.GITHUB_TOKEN }} --repo ${{ github.repository }} --label-pylint automated/pylint --label-cython automated/cython-lint
+          python .github/scripts/run_pylint_check.py --token ${{ secrets.GITHUB_TOKEN }} --repo ${{ github.repository }}

CPPLINT.cfg

@@ -0,0 +1,2 @@
+linelength=120
+filter=-readability/casting,-build/include_subdir

pyproject.toml

@@ -63,6 +63,7 @@ dev = [
 lint = [
     "pylint == 3.0.3",
     "cython-lint == 0.16.0",
+    "cpplint == 1.6.1",
 ]
 
 [tool.pylint.main]

spotfire/cabfile_helpers.c

@@ -4,6 +4,8 @@
 
 #include <Python.h>
 
+#include "cabfile_helpers.h"
+
 #ifdef _WIN32
 
 #include <windows.h>
@@ -16,8 +18,7 @@ static wchar_t *_fci_convert_utf_to_wide(const char *utf, int *err) {
     if (obj == NULL) {
         if (PyErr_ExceptionMatches(PyExc_MemoryError)) {
             *err = ENOMEM;
-        }
-        else {
+        } else {
             *err = EINVAL;
         }
         PyErr_Clear();
@@ -75,7 +76,7 @@ FNFCICLOSE(_fci_cb_close) {
 }
 
 FNFCISEEK(_fci_cb_seek) {
-    long result = (long)_lseek((int)hf, dist, seektype);
+    long result = (long)_lseek((int)hf, dist, seektype);  /* NOLINT(runtime/int) */
     if (result == -1)
         *err = errno;
     return result;
@@ -100,7 +101,7 @@ FNFCIFILEPLACED(_fci_cb_file_placed) {
 FNFCIGETTEMPFILE(_fci_cb_get_temp_file) {
     char *name = _tempnam("", "cabtmp");
     if ((name != NULL) && ((int)strlen(name) < cbTempName)) {
-        strcpy(pszTempName, name);
+        strncpy(pszTempName, name, cbTempName);
         free(name);
         return TRUE;
     }
@@ -130,7 +131,7 @@ FNFCIGETOPENINFO(_fci_cb_get_open_info) {
         return -1;
     }
     FILETIME ft;
-    if(GetFileTime(handle, NULL, NULL, &ft) == FALSE) {
+    if (GetFileTime(handle, NULL, NULL, &ft) == FALSE) {
         CloseHandle(handle);
         PyMem_Free(wide);
         return -1;

spotfire/cabfile_helpers.h

@@ -2,6 +2,9 @@
    This file is subject to the license terms contained
    in the license file that is distributed with this file. */
 
+#ifndef SPOTFIRE_CABFILE_HELPERS_H_
+#define SPOTFIRE_CABFILE_HELPERS_H_
+
 extern FNFCIALLOC(_fci_cb_alloc);
 extern FNFCIFREE(_fci_cb_free);
 extern FNFCIOPEN(_fci_cb_open);
@@ -15,3 +18,5 @@ extern FNFCIGETTEMPFILE(_fci_cb_get_temp_file);
 extern FNFCISTATUS(_fci_cb_status);
 extern FNFCIGETNEXTCABINET(_fci_cb_get_next_cabinet);
 extern FNFCIGETOPENINFO(_fci_cb_get_open_info);
+
+#endif /* SPOTFIRE_CABFILE_HELPERS_H_ */

spotfire/sbdf_helpers.c

@@ -15,17 +15,17 @@ FILE *_pathlike_to_fileptr(PyObject *file, const char* mode) {
     PyObject *filename_obj;
 
     /* int: use the given file descriptor */
-    if(PyLong_Check(file)) {
+    if (PyLong_Check(file)) {
         fd = PyObject_AsFileDescriptor(file);
-        if(fd == -1) return NULL;
+        if (fd == -1) return NULL;
         the_file = fdopen(fd, mode);
     /* bytes: use the given file name */
-    } else if(PyBytes_Check(file)) {
+    } else if (PyBytes_Check(file)) {
         filename = PyBytes_AsString(file);
         the_file = fopen(filename, mode);
     /* unicode/str: decode the given filename as utf-8 */
-    } else if(PyUnicode_Check(file)) {
-        if(!PyUnicode_FSConverter(file, &filename_obj)) return NULL;
+    } else if (PyUnicode_Check(file)) {
+        if (!PyUnicode_FSConverter(file, &filename_obj)) return NULL;
         filename = PyBytes_AsString(filename_obj);
         the_file = fopen(filename, mode);
         Py_XDECREF(filename_obj);
@@ -34,7 +34,7 @@ FILE *_pathlike_to_fileptr(PyObject *file, const char* mode) {
         PyErr_SetString(PyExc_TypeError, "str, bytes, or integer argument expected");
     }
 
-    if(the_file == NULL) {
+    if (the_file == NULL) {
         PyErr_SetFromErrno(PyExc_IOError);
     }
     return the_file;
@@ -56,7 +56,7 @@ void _allocated_list_add(struct _AllocatedList *alist, void *allocated) {
 }
 
 void _allocated_list_done(struct _AllocatedList *alist, _allocated_dealloc_fn fun) {
-    for(int i = 0; i < alist->count; i++) {
+    for (int i = 0; i < alist->count; i++) {
         fun(alist->allocated[i]);
         alist->allocated[i] = NULL;
     }
@@ -69,41 +69,40 @@ sbdf_object *_export_extract_string_obj(PyObject *vals, PyObject *invalids, Py_s
     t->type = sbdf_vt_string();
     t->count = (int)count;
     char **data = (char **)calloc(count, sizeof(char *));
-    if (!data)
-    {
+    if (!data) {
         PyErr_Format(PyExc_MemoryError, "memory exhausted");
         sbdf_obj_destroy(t);
         return NULL;
     }
     t->data = data;
 
-    for(int i = 0; i < count; i++) {
+    for (int i = 0; i < count; i++) {
         Py_ssize_t idx = start + i;
         PyObject *inv = PySequence_GetItem(invalids, idx);
-        if(inv == NULL) {
+        if (inv == NULL) {
             sbdf_obj_destroy(t);
             return NULL;
         }
-        if(PyObject_IsTrue(inv)) {
+        if (PyObject_IsTrue(inv)) {
             /* true: invalid value, add empty value to t->data */
             data[i] = sbdf_str_create_len("", 0);
         } else {
             /* false: valid value, add encoded value to t->data */
             PyObject *val = PySequence_GetItem(vals, idx);
-            if(val == NULL) {
+            if (val == NULL) {
                 Py_XDECREF(inv);
                 sbdf_obj_destroy(t);
                 return NULL;
             }
             PyObject *val_str = PyObject_Str(val);
-            if(val_str == NULL) {
+            if (val_str == NULL) {
                 Py_XDECREF(val);
                 Py_XDECREF(inv);
                 sbdf_obj_destroy(t);
                 return NULL;
             }
             PyObject *val_encoded = PyObject_CallMethod(val_str, "encode", "s", "utf-8");
-            if(val_encoded == NULL) {
+            if (val_encoded == NULL) {
                 Py_XDECREF(val_str);
                 Py_XDECREF(val);
                 Py_XDECREF(inv);
@@ -112,7 +111,7 @@ sbdf_object *_export_extract_string_obj(PyObject *vals, PyObject *invalids, Py_s
             }
             char *val_buf;
             Py_ssize_t val_len;
-            if(PyBytes_AsStringAndSize(val_encoded, &val_buf, &val_len) == -1) {
+            if (PyBytes_AsStringAndSize(val_encoded, &val_buf, &val_len) == -1) {
                 Py_XDECREF(val_encoded);
                 Py_XDECREF(val_str);
                 Py_XDECREF(val);
@@ -137,33 +136,32 @@ sbdf_object *_export_extract_binary_obj(PyObject *vals, PyObject *invalids, Py_s
     t->type = sbdf_vt_binary();
     t->count = (int)count;
     unsigned char **data = (unsigned char **)calloc(count, sizeof(unsigned char *));
-    if (!data)
-    {
+    if (!data) {
         PyErr_Format(PyExc_MemoryError, "memory exhausted");
         sbdf_obj_destroy(t);
         return NULL;
     }
     t->data = data;
 
-    for(int i = 0; i < count; i++) {
+    for (int i = 0; i < count; i++) {
         Py_ssize_t idx = start + i;
         PyObject *inv = PySequence_GetItem(invalids, idx);
-        if(inv == NULL) {
+        if (inv == NULL) {
             sbdf_obj_destroy(t);
             return NULL;
         }
-        if(PyObject_IsTrue(inv)) {
+        if (PyObject_IsTrue(inv)) {
             /* true: invalid value, add empty value to t->data */
             data[i] = sbdf_ba_create(0, 0);
         } else {
             /* false: valid value, add value to t->data */
             PyObject *val = PySequence_GetItem(vals, idx);
-            if(val == NULL) {
+            if (val == NULL) {
                 Py_XDECREF(inv);
                 sbdf_obj_destroy(t);
                 return NULL;
             }
-            if(!PyBytes_Check(val)) {
+            if (!PyBytes_Check(val)) {
                 PyErr_Format(PyExc_SBDFError, "cannot convert '%S' to Spotfire Binary type; incompatible types", val);
                 Py_XDECREF(val);
                 Py_XDECREF(inv);
@@ -172,7 +170,7 @@ sbdf_object *_export_extract_binary_obj(PyObject *vals, PyObject *invalids, Py_s
             }
             char *val_buf;
             Py_ssize_t val_len;
-            if(PyBytes_AsStringAndSize(val, &val_buf, &val_len) == -1) {
+            if (PyBytes_AsStringAndSize(val, &val_buf, &val_len) == -1) {
                 Py_XDECREF(val);
                 Py_XDECREF(inv);
                 sbdf_obj_destroy(t);

spotfire/sbdf_helpers.h

@@ -2,6 +2,9 @@
    This file is subject to the license terms contained
    in the license file that is distributed with this file. */
 
+#ifndef SPOTFIRE_SBDF_HELPERS_H_
+#define SPOTFIRE_SBDF_HELPERS_H_
+
 #include <Python.h>
 #include <stdio.h>
 #include <all.h>
@@ -35,3 +38,5 @@ struct _SbdfDecimal {
 /* Utility functions for extracting strings from Python ``Union[str,bytes]`` into C */
 extern sbdf_object *_export_extract_string_obj(PyObject *vals, PyObject *invalids, Py_ssize_t start, Py_ssize_t count);
 extern sbdf_object *_export_extract_binary_obj(PyObject *vals, PyObject *invalids, Py_ssize_t start, Py_ssize_t count);
+
+#endif /* SPOTFIRE_SBDF_HELPERS_H_ */