tools: add cpplint rule for NULL usage

This commit is a suggestion for adding a rule for NULL usages in the code base. This will currently report a number of errors which could be ignored using // NOLINT (readability/null_usage) PR-URL: nodejs#17373 Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
juanarbol · Dec 1, 2017 · 12c8b4d · 12c8b4d
1 parent e32bbbf
commit 12c8b4d
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 19 deletions.
diff --git a/src/node.h b/src/node.h
@@ -501,13 +501,13 @@ extern "C" NODE_EXTERN void node_module_register(void* mod);
     {                                                                 \
       NODE_MODULE_VERSION,                                            \
       flags,                                                          \
-      NULL,                                                           \
+      NULL,  /* NOLINT (readability/null_usage) */                    \
       __FILE__,                                                       \
       (node::addon_register_func) (regfunc),                          \
-      NULL,                                                           \
+      NULL,  /* NOLINT (readability/null_usage) */                    \
       NODE_STRINGIFY(modname),                                        \
       priv,                                                           \
-      NULL                                                            \
+      NULL   /* NOLINT (readability/null_usage) */                    \
     };                                                                \
     NODE_C_CTOR(_register_ ## modname) {                              \
       node_module_register(&_module);                                 \
@@ -520,23 +520,24 @@ extern "C" NODE_EXTERN void node_module_register(void* mod);
     {                                                                 \
       NODE_MODULE_VERSION,                                            \
       flags,                                                          \
-      NULL,                                                           \
+      NULL,  /* NOLINT (readability/null_usage) */                    \
       __FILE__,                                                       \
-      NULL,                                                           \
+      NULL,  /* NOLINT (readability/null_usage) */                    \
       (node::addon_context_register_func) (regfunc),                  \
       NODE_STRINGIFY(modname),                                        \
       priv,                                                           \
-      NULL                                                            \
+      NULL  /* NOLINT (readability/null_usage) */                     \
     };                                                                \
     NODE_C_CTOR(_register_ ## modname) {                              \
       node_module_register(&_module);                                 \
     }                                                                 \
   }
 
 #define NODE_MODULE(modname, regfunc)                                 \
-  NODE_MODULE_X(modname, regfunc, NULL, 0)
+  NODE_MODULE_X(modname, regfunc, NULL, 0)  // NOLINT (readability/null_usage)
 
 #define NODE_MODULE_CONTEXT_AWARE(modname, regfunc)                   \
+  /* NOLINTNEXTLINE (readability/null_usage) */                       \
   NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, 0)
 
 /*

diff --git a/src/node_api.h b/src/node_api.h
@@ -100,7 +100,7 @@ typedef struct {
   EXTERN_C_END
 
 #define NAPI_MODULE(modname, regfunc) \
-  NAPI_MODULE_X(modname, regfunc, NULL, 0)
+  NAPI_MODULE_X(modname, regfunc, NULL, 0)  // NOLINT (readability/null_usage)
 
 #define NAPI_AUTO_LENGTH SIZE_MAX
 

diff --git a/src/node_win32_etw_provider-inl.h b/src/node_win32_etw_provider-inl.h
@@ -115,7 +115,7 @@ extern int events_enabled;
   DWORD status = event_write(node_provider,                                   \
                              &eventDescriptor,                                \
                              0,                                               \
-                             NULL);                                           \
+                             NULL);  // NOLINT (readability/null_usage)       \
   CHECK_EQ(status, ERROR_SUCCESS);
 
 

diff --git a/test/addons-napi/7_factory_wrap/binding.cc b/test/addons-napi/7_factory_wrap/binding.cc
@@ -16,6 +16,7 @@ napi_value Init(napi_env env, napi_value exports) {
   NAPI_CALL(env, MyObject::Init(env));
 
   NAPI_CALL(env,
+      // NOLINTNEXTLINE (readability/null_usage)
       napi_create_function(env, "exports", -1, CreateObject, NULL, &exports));
   return exports;
 }

diff --git a/test/addons-napi/test_make_callback/binding.cc b/test/addons-napi/test_make_callback/binding.cc
@@ -8,6 +8,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) {
   const int kMaxArgs = 10;
   size_t argc = kMaxArgs;
   napi_value args[kMaxArgs];
+  // NOLINTNEXTLINE (readability/null_usage)
   NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
 
   NAPI_ASSERT(env, argc > 0, "Wrong number of arguments");
@@ -47,6 +48,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) {
 napi_value Init(napi_env env, napi_value exports) {
   napi_value fn;
   NAPI_CALL(env, napi_create_function(
+      // NOLINTNEXTLINE (readability/null_usage)
       env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn));
   NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn));
   return exports;

diff --git a/test/addons-napi/test_make_callback_recurse/binding.cc b/test/addons-napi/test_make_callback_recurse/binding.cc
@@ -7,6 +7,7 @@ namespace {
 napi_value MakeCallback(napi_env env, napi_callback_info info) {
   size_t argc = 2;
   napi_value args[2];
+  // NOLINTNEXTLINE (readability/null_usage)
   NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
 
   napi_value recv = args[0];
@@ -21,6 +22,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) {
 napi_value Init(napi_env env, napi_value exports) {
   napi_value fn;
   NAPI_CALL(env, napi_create_function(
+      // NOLINTNEXTLINE (readability/null_usage)
       env, NULL, NAPI_AUTO_LENGTH, MakeCallback, NULL, &fn));
   NAPI_CALL(env, napi_set_named_property(env, exports, "makeCallback", fn));
   return exports;

diff --git a/tools/cpplint.py b/tools/cpplint.py
@@ -499,7 +499,6 @@
 _ALT_TOKEN_REPLACEMENT_PATTERN = re.compile(
     r'[ =()](' + ('|'.join(_ALT_TOKEN_REPLACEMENT.keys())) + r')(?=[ (]|$)')
 
-
 # These constants define types of headers for use with
 # _IncludeState.CheckNextIncludeOrder().
 _C_SYS_HEADER = 1
@@ -526,6 +525,8 @@
 # Match string that indicates we're working on a Linux Kernel file.
 _SEARCH_KERNEL_FILE = re.compile(r'\b(?:LINT_KERNEL_FILE)')
 
+_NULL_TOKEN_PATTERN = re.compile(r'\bNULL\b')
+
 _regexp_compile_cache = {}
 
 # {str, set(int)}: a map from error categories to sets of linenumbers
@@ -4156,6 +4157,27 @@ def CheckAltTokens(filename, clean_lines, linenum, error):
           'Use operator %s instead of %s' % (
               _ALT_TOKEN_REPLACEMENT[match.group(1)], match.group(1)))
 
+def CheckNullTokens(filename, clean_lines, linenum, error):
+  """Check NULL usage.
+
+  Args:
+    filename: The name of the current file.
+    clean_lines: A CleansedLines instance containing the file.
+    linenum: The number of the line to check.
+    error: The function to call with any errors found.
+  """
+  line = clean_lines.elided[linenum]
+
+  # Avoid preprocessor lines
+  if Match(r'^\s*#', line):
+    return
+
+  if line.find('/*') >= 0 or line.find('*/') >= 0:
+    return
+
+  for match in _NULL_TOKEN_PATTERN.finditer(line):
+    error(filename, linenum, 'readability/null_usage', 2,
+          'Use nullptr instead of NULL')
 
 def GetLineWidth(line):
   """Determines the width of the line in column positions.
@@ -4294,6 +4316,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state,
   CheckSpacingForFunctionCall(filename, clean_lines, linenum, error)
   CheckCheck(filename, clean_lines, linenum, error)
   CheckAltTokens(filename, clean_lines, linenum, error)
+  CheckNullTokens(filename, clean_lines, linenum, error)
   classinfo = nesting_state.InnermostClass()
   if classinfo:
     CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error)

diff --git a/tools/icu/iculslocs.cc b/tools/icu/iculslocs.cc
@@ -231,14 +231,14 @@ int dumpAllButInstalledLocales(int lev,
 int list(const char* toBundle) {
   UErrorCode status = U_ZERO_ERROR;
 
-  FILE* bf = NULL;
+  FILE* bf = NULL;  // NOLINT (readability/null_usage)
 
-  if (toBundle != NULL) {
+  if (toBundle != NULL) {  // NOLINT (readability/null_usage)
     if (VERBOSE) {
       printf("writing to bundle %s\n", toBundle);
     }
     bf = fopen(toBundle, "wb");
-    if (bf == NULL) {
+    if (bf == NULL) {  // NOLINT (readability/null_usage)
       printf("ERROR: Could not open '%s' for writing.\n", toBundle);
       return 1;
     }
@@ -258,6 +258,7 @@ int list(const char* toBundle) {
       ures_openDirect(packageName.data(), locale, &status));
   ASSERT_SUCCESS(&status, "while opening the bundle");
   LocalUResourceBundlePointer installedLocales(
+      // NOLINTNEXTLINE (readability/null_usage)
       ures_getByKey(bund.getAlias(), INSTALLEDLOCALES, NULL, &status));
   ASSERT_SUCCESS(&status, "while fetching installed locales");
 
@@ -266,7 +267,7 @@ int list(const char* toBundle) {
     printf("Locales: %d\n", count);
   }
 
-  if (bf != NULL) {
+  if (bf != NULL) {  // NOLINT (readability/null_usage)
     // write the HEADER
     fprintf(bf,
             "// Warning this file is automatically generated\n"
@@ -310,17 +311,17 @@ int list(const char* toBundle) {
 
     UBool exists;
     if (localeExists(key, &exists)) {
-      if (bf != NULL) fclose(bf);
+      if (bf != NULL) fclose(bf);  // NOLINT (readability/null_usage)
       return 1;  // get out.
     }
     if (exists) {
       validCount++;
       printf("%s\n", key);
-      if (bf != NULL) {
+      if (bf != NULL) {  // NOLINT (readability/null_usage)
         fprintf(bf, "        %s {\"\"}\n", key);
       }
     } else {
-      if (bf != NULL) {
+      if (bf != NULL) {  // NOLINT (readability/null_usage)
         fprintf(bf, "//      %s {\"\"}\n", key);
       }
       if (VERBOSE) {
@@ -329,7 +330,7 @@ int list(const char* toBundle) {
     }
   }
 
-  if (bf != NULL) {
+  if (bf != NULL) {  // NOLINT (readability/null_usage)
     fprintf(bf, "    } // %d/%d valid\n", validCount, count);
     // write the HEADER
     fprintf(bf, "}\n");
@@ -371,7 +372,7 @@ int main(int argc, const char* argv[]) {
       usage();
       return 0;
     } else if (!strcmp(arg, "-l")) {
-      if (list(NULL)) {
+      if (list(NULL)) {  // NOLINT (readability/null_usage)
         return 1;
       }
     } else if (!strcmp(arg, "-b") && (argsLeft >= 1)) {