Merge #36859 refactor: clint.py => clint.lua

.clang-tidy

@@ -3,6 +3,10 @@ Checks: >
   Enable all warnings by default. This ensures we don't miss new and useful
   warnings when a new version of clang-tidy is dropped.
 
+  To list all available checks:
+
+      clang-tidy --list-checks --checks="*"
+
   IMPORTANT
   clang-tidy doesn't support comments but we can simulate comments by just
   writing text directly here. These are then interpreted as warnings and will
@@ -55,12 +59,14 @@ Checks: >
   -misc-misplaced-const,
   -misc-no-recursion,
   -performance-no-int-to-ptr,
+  -portability-avoid-pragma-once,
   -readability-function-cognitive-complexity,
   -readability-identifier-length,
   -readability-magic-numbers,
   -readability-math-missing-parentheses,
   -readability-redundant-declaration, Conflicts with our header generation scripts,
   -readability-suspicious-call-argument,
+  -readability-use-concise-preprocessor-directives,
 
   Aliases. These are just duplicates of other warnings and should always be ignored,
   -bugprone-narrowing-conversions,

.github/workflows/test.yml

@@ -70,7 +70,7 @@ jobs:
         run: cmake --build build --target lintsh
 
       - if: success() || failure() && steps.abort_job.outputs.status == 'success'
-        name: clint.py
+        name: clint.lua
         run: cmake --build build --target lintc-clint
 
       - if: success() || failure() && steps.abort_job.outputs.status == 'success'

runtime/doc/dev_style.txt

@@ -119,9 +119,9 @@ Nvim-Specific Magic
 
 clint ~
 
-Use `clint.py` to detect style errors.
+Use `clint.lua` to detect style errors.
 
-`src/clint.py` is a Python script that reads a source file and identifies
+`src/clint.lua` is a Lua script that reads a source file and identifies
 style errors. It is not perfect, and has both false positives and false
 negatives, but it is still a valuable tool. False positives can be ignored by
 putting `// NOLINT` at the end of the line.
@@ -129,7 +129,7 @@ putting `// NOLINT` at the end of the line.
 uncrustify ~
 
 src/uncrustify.cfg is the authority for expected code formatting, for cases
-not covered by clint.py.  We remove checks in clint.py if they are covered by
+not covered by clint.lua.  We remove checks in clint.lua if they are covered by
 uncrustify rules.
 
 ==============================================================================

src/nvim/CMakeLists.txt

@@ -928,12 +928,13 @@ else()
 endif()
 add_glob_target(
   TARGET lintc-clint
-  COMMAND ${PROJECT_SOURCE_DIR}/src/clint.py
-  FLAGS --output=${LINT_OUTPUT_FORMAT}
+  COMMAND $<TARGET_FILE:nvim_bin>
+  FLAGS --clean -l ${PROJECT_SOURCE_DIR}/src/clint.lua --output=${LINT_OUTPUT_FORMAT}
   FILES ${LINT_NVIM_SOURCES}
   EXCLUDE
     tui/terminfo_defs.h
     xxd/xxd.c)
+add_dependencies(lintc-clint nvim_bin)
 
 set(UNCRUSTIFY_PRG ${DEPS_BIN_DIR}/uncrustify)
 set(UNCRUSTIFY_CONFIG ${PROJECT_SOURCE_DIR}/src/uncrustify.cfg)

src/nvim/eval/typval.c

@@ -116,8 +116,8 @@ bool tv_in_free_unref_items = false;
 
 const char *const tv_empty_string = "";
 
-//{{{1 Lists
-//{{{2 List item
+// Lists:
+// List item:
 
 /// Allocate a list item
 ///
@@ -150,7 +150,7 @@ listitem_T *tv_list_item_remove(list_T *const l, listitem_T *const item)
   return next_item;
 }
 
-//{{{2 List watchers
+// List watchers:
 
 /// Add a watcher to a list
 ///
@@ -198,7 +198,7 @@ static void tv_list_watch_fix(list_T *const l, const listitem_T *const item)
   }
 }
 
-//{{{2 Alloc/free
+// Alloc/free:
 
 /// Allocate an empty list
 ///
@@ -335,7 +335,7 @@ void tv_list_unref(list_T *const l)
   }
 }
 
-//{{{2 Add/remove
+// Add/remove:
 
 /// Remove items "item" to "item2" from list "l"
 ///
@@ -578,7 +578,7 @@ void tv_list_append_number(list_T *const l, const varnumber_T n)
   });
 }
 
-//{{{2 Operations on the whole list
+// Operations on the whole list:
 
 /// Make a copy of list
 ///
@@ -1569,7 +1569,7 @@ void tv_list_reverse(list_T *const l)
   l->lv_idx = l->lv_len - l->lv_idx - 1;
 }
 
-//{{{2 Indexing/searching
+// Indexing/searching:
 
 /// Locate item with a given index in a list and return it
 ///
@@ -1719,8 +1719,8 @@ int tv_list_idx_of_item(const list_T *const l, const listitem_T *const item)
   return -1;
 }
 
-//{{{1 Dictionaries
-//{{{2 Dictionary watchers
+// Dictionaries:
+// Dictionary watchers:
 
 /// Perform all necessary cleanup for a `DictWatcher` instance
 ///
@@ -2014,7 +2014,7 @@ void tv_dict_watcher_notify(dict_T *const dict, const char *const key, typval_T
   }
 }
 
-//{{{2 Dictionary item
+// Dictionary item:
 
 /// Allocate a dictionary item
 ///
@@ -2094,7 +2094,7 @@ void tv_dict_item_remove(dict_T *const dict, dictitem_T *const item)
   tv_dict_item_free(item);
 }
 
-//{{{2 Alloc/free
+// Alloc/free:
 
 /// Allocate an empty dictionary.
 /// Caller should take care of the reference count.
@@ -2204,7 +2204,7 @@ void tv_dict_unref(dict_T *const d)
   }
 }
 
-//{{{2 Indexing/searching
+// Indexing/searching:
 
 /// Find item in dictionary
 ///
@@ -2428,7 +2428,7 @@ int tv_dict_wrong_func_name(dict_T *d, typval_T *tv, const char *name)
          && var_wrong_func_name(name, true);
 }
 
-//{{{2 dict_add*
+// dict_add*:
 
 /// Add item to dictionary
 ///
@@ -2659,7 +2659,7 @@ int tv_dict_add_func(dict_T *const d, const char *const key, const size_t key_le
   return OK;
 }
 
-//{{{2 Operations on the whole dict
+// Operations on the whole dict:
 
 /// Clear all the keys of a Dictionary. "d" remains a valid empty Dictionary.
 ///
@@ -2868,8 +2868,8 @@ void tv_dict_set_keys_readonly(dict_T *const dict)
   });
 }
 
-//{{{1 Blobs
-//{{{2 Alloc/free
+// Blobs:
+// Alloc/free:
 
 /// Allocate an empty blob.
 ///
@@ -2906,7 +2906,7 @@ void tv_blob_unref(blob_T *const b)
   }
 }
 
-//{{{2 Operations on the whole blob
+// Operations on the whole blob:
 
 /// Check whether two blobs are equal.
 ///
@@ -3172,9 +3172,9 @@ void f_list2blob(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   });
 }
 
-//{{{1 Generic typval operations
-//{{{2 Init/alloc/clear
-//{{{3 Alloc
+// Generic typval operations:
+// Init/alloc/clear:
+// Alloc:
 
 /// Allocate an empty list for a return value
 ///
@@ -3367,7 +3367,7 @@ void tv_blob_copy(blob_T *const from, typval_T *const to)
   }
 }
 
-//{{{3 Clear
+// Clear:
 #define TYPVAL_ENCODE_ALLOW_SPECIALS false
 #define TYPVAL_ENCODE_CHECK_BEFORE
 
@@ -3634,7 +3634,7 @@ void tv_clear(typval_T *const tv)
   assert(evn_ret == OK);
 }
 
-//{{{3 Free
+// Free:
 
 /// Free allocated Vimscript object and value stored inside
 ///
@@ -3674,7 +3674,7 @@ void tv_free(typval_T *tv)
   xfree(tv);
 }
 
-//{{{3 Copy
+// Copy:
 
 /// Copy typval from one location to another
 ///
@@ -3730,7 +3730,7 @@ void tv_copy(const typval_T *const from, typval_T *const to)
   }
 }
 
-//{{{2 Locks
+// Locks:
 
 /// Lock or unlock an item
 ///
@@ -3909,7 +3909,7 @@ bool value_check_lock(VarLockStatus lock, const char *name, size_t name_len)
   return true;
 }
 
-//{{{2 Comparison
+// Comparison:
 
 static int tv_equal_recurse_limit;
 
@@ -4000,7 +4000,7 @@ bool tv_equal(typval_T *const tv1, typval_T *const tv2, const bool ic)
   return false;
 }
 
-//{{{2 Type checks
+// Type checks:
 
 /// Check that given value is a number or string
 ///
@@ -4137,7 +4137,7 @@ bool tv_check_str(const typval_T *const tv)
   return false;
 }
 
-//{{{2 Get
+// Get:
 
 /// Get the number value of a Vimscript object
 ///

src/nvim/path.c

@@ -55,22 +55,22 @@ FileComparison path_full_compare(char *const s1, char *const s2, const bool chec
                                  const bool expandenv)
   FUNC_ATTR_NONNULL_ALL
 {
-  char exp1[MAXPATHL];
+  char expanded1[MAXPATHL];
   char full1[MAXPATHL];
   char full2[MAXPATHL];
   FileID file_id_1, file_id_2;
 
   if (expandenv) {
-    expand_env(s1, exp1, MAXPATHL);
+    expand_env(s1, expanded1, MAXPATHL);
   } else {
-    xstrlcpy(exp1, s1, MAXPATHL);
+    xstrlcpy(expanded1, s1, MAXPATHL);
   }
-  bool id_ok_1 = os_fileid(exp1, &file_id_1);
+  bool id_ok_1 = os_fileid(expanded1, &file_id_1);
   bool id_ok_2 = os_fileid(s2, &file_id_2);
   if (!id_ok_1 && !id_ok_2) {
     // If os_fileid() doesn't work, may compare the names.
     if (checkname) {
-      vim_FullName(exp1, full1, MAXPATHL, false);
+      vim_FullName(expanded1, full1, MAXPATHL, false);
       vim_FullName(s2, full2, MAXPATHL, false);
       if (path_fnamecmp(full1, full2) == 0) {
         return kEqualFileNames;

src/uncrustify.cfg

@@ -1028,7 +1028,7 @@ sp_before_for_colon             = ignore   # ignore/add/remove/force
 sp_extern_paren                 = ignore   # ignore/add/remove/force
 
 # Add or remove space after the opening of a C++ comment, as in '// <here> A'.
-sp_cmt_cpp_start                = ignore   # ignore/add/remove/force
+sp_cmt_cpp_start                = add      # ignore/add/remove/force
 
 # remove space after the '//' and the pvs command '-V1234',
 # only works with sp_cmt_cpp_start set to add or force.

test/functional/fixtures/clint_test.c

@@ -0,0 +1,163 @@
+// Test file to trigger all ERROR_CATEGORIES in clint.lua
+// This file contains intentional errors to test the linter
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+
+// build/endif_comment: Uncommented text after #endif
+#ifdef SOME_CONDITION
+# define TEST 1
+#endif SOME_CONDITION
+
+// build/include_defs: Non-defs header included (but this is a .c file, so might not trigger)
+
+// build/printf_format: %q format specifier
+void test_printf_format()
+{
+  printf("%q", "test");  // Should trigger runtime/printf_format
+}
+
+// build/storage_class: Storage class not first
+const static int x = 5;  // Should trigger build/storage_class
+
+// readability/bool: Use TRUE/FALSE instead of true/false
+#define TRUE 1
+#define FALSE 0
+#define MAYBE 2
+
+void test_bool()
+{
+  int flag = TRUE;  // Should trigger readability/bool
+  if (flag == FALSE) {  // Should trigger readability/bool
+    printf("false\n");
+  }
+  int maybe_val = MAYBE;  // Should trigger readability/bool
+}
+
+// readability/multiline_comment: Complex multi-line comment
+void test_multiline_comment()
+{
+  /* This is a multi-line
+     comment that spans
+     multiple lines and doesn't close properly on the same line */
+}
+
+// readability/nul: NUL byte in file (can't easily test this in text)
+
+// readability/utf8: Invalid UTF-8 (can't easily test)
+
+// readability/increment: Pre-increment in statements
+void test_increment()
+{
+  int i = 0;
+  ++i;  // Should trigger readability/increment
+  for (int j = 0; j < 10; ++j) {  // Should trigger readability/increment
+    printf("%d\n", j);
+  }
+}
+
+// runtime/arrays: Variable-length arrays
+void test_arrays(int size)
+{
+  int arr[size];  // Should trigger runtime/arrays
+}
+
+// runtime/int: Use C basic types instead of fixed-width
+void test_int_types()
+{
+  short x = 1;        // Should trigger runtime/int
+  long long y = 2;    // Should trigger runtime/int
+}
+
+// runtime/memset: memset with wrong arguments
+void test_memset()
+{
+  char buf[100];
+  memset(buf, sizeof(buf), 0);  // Should trigger runtime/memset
+}
+
+// runtime/printf: Use sprintf instead of snprintf
+void test_printf()
+{
+  char buf[100];
+  sprintf(buf, "test");  // Should trigger runtime/printf
+}
+
+// runtime/printf_format: %N$ formats
+void test_printf_format2()
+{
+  printf("%1$d", 42);  // Should trigger runtime/printf_format
+}
+
+// runtime/threadsafe_fn: Use non-thread-safe functions
+void test_threading()
+{
+  time_t t;
+  char *time_str = ctime(&t);  // Should trigger runtime/threadsafe_fn
+  asctime(localtime(&t));      // Should trigger runtime/threadsafe_fn
+}
+
+// runtime/deprecated: (This might be Neovim-specific)
+
+// whitespace/comments: Missing space after //
+void test_comments()
+{
+  int x = 5;  // This is a comment  // Should trigger whitespace/comments
+}
+
+// whitespace/indent: (Hard to test in this format)
+
+// whitespace/operators: (Hard to test)
+
+// whitespace/cast: (Hard to test)
+
+// build/init_macro: INIT() macro in non-header (but this is a .c file)
+
+// build/header_guard: No #pragma once (but this is a .c file)
+
+// build/defs_header: extern variables in _defs.h (but this is a .c file)
+
+// readability/old_style_comment: Old-style /* */ comment
+void test_old_style_comment()
+{
+  int x = 5; /* This is an old-style comment */  // Should trigger readability/old_style_comment
+}
+
+// Try to trigger more categories
+void test_more()
+{
+  // Try strcpy and strncpy
+  char dest[100];
+  char src[] = "test";
+  strcpy(dest, src);   // Should trigger runtime/printf
+  strncpy(dest, src, sizeof(dest));  // Should trigger runtime/printf
+
+  // Try malloc and free (should trigger runtime/memory_fn)
+  int *ptr = malloc(sizeof(int));  // Should trigger runtime/memory_fn
+  free(ptr);  // Should trigger runtime/memory_fn
+
+  // Try getenv and setenv
+  char *env = getenv("HOME");  // Should trigger runtime/os_fn
+  setenv("TEST", "value", 1);   // Should trigger runtime/os_fn
+}
+
+int main()
+{
+  test_printf_format();
+  test_bool();
+  test_multiline_comment();
+  test_multiline_string();
+  test_increment();
+  test_arrays(10);
+  test_int_types();
+  test_memset();
+  test_printf();
+  test_printf_format2();
+  test_threading();
+  test_comments();
+  test_old_style_comment();
+  test_more();
+
+  return 0;
+}

test/functional/script/clint_spec.lua

@@ -0,0 +1,50 @@
+local t = require('test.testutil')
+local n = require('test.functional.testnvim')()
+
+describe('clint.lua', function()
+  local clint_path = 'src/clint.lua'
+  local test_file = 'test/functional/fixtures/clint_test.c'
+
+  local function run_clint(filepath)
+    local proc = n.spawn_wait('-l', clint_path, filepath)
+    local output = proc:output()
+    local lines = vim.split(output, '\n', { plain = true, trimempty = true })
+    return lines
+  end
+
+  it('a linter lints', function()
+    local output_lines = run_clint(test_file)
+    local expected = {
+      'test/functional/fixtures/clint_test.c:11:  Uncommented text after #endif is non-standard.  Use a comment.  [build/endif_comment] [5]',
+      'test/functional/fixtures/clint_test.c:18:  "%q" in format strings is deprecated.  Use "%" PRId64 instead.  [runtime/printf_format] [3]',
+      'test/functional/fixtures/clint_test.c:22:  Storage class (static, extern, typedef, etc) should be first.  [build/storage_class] [5]',
+      'test/functional/fixtures/clint_test.c:25:  Use true instead of TRUE.  [readability/bool] [4]',
+      'test/functional/fixtures/clint_test.c:26:  Use false instead of FALSE.  [readability/bool] [4]',
+      'test/functional/fixtures/clint_test.c:27:  Use kNONE from TriState instead of MAYBE.  [readability/bool] [4]',
+      'test/functional/fixtures/clint_test.c:31:  Use true instead of TRUE.  [readability/bool] [4]',
+      'test/functional/fixtures/clint_test.c:32:  Use false instead of FALSE.  [readability/bool] [4]',
+      'test/functional/fixtures/clint_test.c:35:  Use kNONE from TriState instead of MAYBE.  [readability/bool] [4]',
+      'test/functional/fixtures/clint_test.c:41:  /*-style comment found, it should be replaced with //-style.  /*-style comments are only allowed inside macros.  Note that you should not use /*-style comments to document macros itself, use doxygen-style comments for this.  [readability/old_style_comment] [5]',
+      'test/functional/fixtures/clint_test.c:54:  Do not use preincrement in statements, use postincrement instead  [readability/increment] [5]',
+      'test/functional/fixtures/clint_test.c:55:  Do not use preincrement in statements, including for(;; action)  [readability/increment] [4]',
+      "test/functional/fixtures/clint_test.c:63:  Do not use variable-length arrays.  Use an appropriately named ('k' followed by CamelCase) compile-time constant for the size.  [runtime/arrays] [1]",
+      'test/functional/fixtures/clint_test.c:69:  Use int16_t/int64_t/etc, rather than the C type short  [runtime/int] [4]',
+      'test/functional/fixtures/clint_test.c:70:  Use int16_t/int64_t/etc, rather than the C type long long  [runtime/int] [4]',
+      'test/functional/fixtures/clint_test.c:77:  Did you mean "memset(buf, 0, sizeof(buf))"?  [runtime/memset] [4]',
+      'test/functional/fixtures/clint_test.c:84:  Use snprintf instead of sprintf.  [runtime/printf] [5]',
+      'test/functional/fixtures/clint_test.c:90:  %N$ formats are unconventional.  Try rewriting to avoid them.  [runtime/printf_format] [2]',
+      'test/functional/fixtures/clint_test.c:97:  Use os_ctime_r(...) instead of ctime(...). If it is missing, consider implementing it; see os_localtime_r for an example.  [runtime/threadsafe_fn] [2]',
+      'test/functional/fixtures/clint_test.c:98:  Use os_asctime_r(...) instead of asctime(...). If it is missing, consider implementing it; see os_localtime_r for an example.  [runtime/threadsafe_fn] [2]',
+      'test/functional/fixtures/clint_test.c:98:  Use os_localtime_r(...) instead of localtime(...). If it is missing, consider implementing it; see os_localtime_r for an example.  [runtime/threadsafe_fn] [2]',
+      'test/functional/fixtures/clint_test.c:124:  /*-style comment found, it should be replaced with //-style.  /*-style comments are only allowed inside macros.  Note that you should not use /*-style comments to document macros itself, use doxygen-style comments for this.  [readability/old_style_comment] [5]',
+      'test/functional/fixtures/clint_test.c:133:  Use xstrlcpy, xmemcpyz or snprintf instead of strcpy  [runtime/printf] [4]',
+      'test/functional/fixtures/clint_test.c:134:  Use xstrlcpy, xmemcpyz or snprintf instead of strncpy (unless this is from Vim)  [runtime/printf] [4]',
+      'test/functional/fixtures/clint_test.c:137:  Use xmalloc(...) instead of malloc(...).  [runtime/memory_fn] [2]',
+      'test/functional/fixtures/clint_test.c:138:  Use xfree(...) instead of free(...).  [runtime/memory_fn] [2]',
+      'test/functional/fixtures/clint_test.c:141:  Use os_getenv(...) instead of getenv(...).  [runtime/os_fn] [2]',
+      'test/functional/fixtures/clint_test.c:142:  Use os_setenv(...) instead of setenv(...).  [runtime/os_fn] [2]',
+      'Total errors found: 28',
+    }
+    t.eq(expected, output_lines)
+  end)
+end)