fix(checkhealth): mitigate issues with duplicate healthchecks #15919

muniter · web-flow · commit acd5e831b629 · 2021-10-05T15:37:39.000-07:00
* fix(runtime/health): mitigate issues with duplicate healthchecks Previously if a healthcheck was found as Lua and Vim it was executed both times. This new implementations prefers Lua, therefore if two are found It only runs the Lua one, this way a plugin can mantain both implementations the Lua one with the method `check()` and the autoload function `#check()` (for none HEAD nvim versions). **Note: This will require plugins to use `check()` as the function name, since the autoload function that wraps the lua implementation won't be called** * docs(health): use spaces and don't overuse backtics followup to #15259
diff --git a/runtime/autoload/health.vim b/runtime/autoload/health.vim
@@ -41,8 +41,8 @@ function! health#check(plugin_names) abort
     call setline(1, 'ERROR: No healthchecks found.')
   else
     redraw|echo 'Running healthchecks...'
-    for c in healthchecks
-      let [name, func, type] = c
+    for name in sort(keys(healthchecks))
+      let [func, type] = healthchecks[name]
       let s:output = []
       try
         if func == ''
@@ -176,8 +176,30 @@ function! s:discover_healthchecks() abort
   return s:get_healthcheck('*')
 endfunction
 
-" Returns list of lists [ [{name}, {func}, {type}] ] representing healthchecks
+" Returns Dictionary {name: [func, type], ..} representing healthchecks
 function! s:get_healthcheck(plugin_names) abort
+  let health_list = s:get_healthcheck_list(a:plugin_names)
+  let healthchecks = {}
+  for c in health_list
+    let name = c[0]
+    let existent = get(healthchecks, name, [])
+    " If an entry with the same name exists and is from vim, prefer Lua so
+    " overwrite it.
+    if existent != []
+      if existent[1] == "v"
+        let healthchecks[name] = c[1:] 
+      else
+        continue
+      endif
+    else
+      let healthchecks[name] = c[1:]
+    endif
+  endfor
+  return healthchecks
+endfunction
+
+" Returns list of lists [ [{name}, {func}, {type}] ] representing healthchecks
+function! s:get_healthcheck_list(plugin_names) abort
   let healthchecks = []
   let plugin_names = type('') == type(a:plugin_names)
         \ ? split(a:plugin_names, ' ', v:false)
diff --git a/runtime/doc/pi_health.txt b/runtime/doc/pi_health.txt
@@ -5,74 +5,74 @@ Author: TJ DeVries <devries.timothyj@gmail.com>
                                       Type |gO| to see the table of contents.
 
 ==============================================================================
-Introduction				*health*
+Introduction                            *health*
 
 health.vim is a minimal framework to help with troubleshooting user
 configuration. Nvim ships with healthchecks for configuration, performance,
 python support, ruby support, clipboard support, and more.
 
 To run the healthchecks, use this command: >
 
-	:checkhealth
+        :checkhealth
 <
 Plugin authors are encouraged to write new healthchecks. |health-dev|
 
 ==============================================================================
-Commands				*health-commands*
+Commands                                *health-commands*
 
-					*:checkhealth* *:CheckHealth*
+                                        *:checkhealth* *:CheckHealth*
 :checkhealth          Run all healthchecks.
-					*E5009*
-		Nvim depends on |$VIMRUNTIME|, 'runtimepath' and 'packpath' to
-		find the standard "runtime files" for syntax highlighting,
-		filetype-specific behavior, and standard plugins (including
-		:checkhealth).  If the runtime files cannot be found then
-		those features will not work.
+                                        *E5009*
+                Nvim depends on |$VIMRUNTIME|, 'runtimepath' and 'packpath' to
+                find the standard "runtime files" for syntax highlighting,
+                filetype-specific behavior, and standard plugins (including
+                :checkhealth).  If the runtime files cannot be found then
+                those features will not work.
 
 :checkhealth {plugins}
-		Run healthcheck(s) for one or more plugins. E.g. to run only
-		the standard Nvim healthcheck: >
-			:checkhealth nvim
+                Run healthcheck(s) for one or more plugins. E.g. to run only
+                the standard Nvim healthcheck: >
+                        :checkhealth nvim
 <
-		To run the healthchecks for the "foo" and "bar" plugins
-		(assuming these plugins are on 'runtimepath' or 'packpath' and
-		they have implemented the Lua or Vimscript interface
-		require("foo.health").check() and health#bar#check(),
-		respectively): >
-			:checkhealth foo bar
+                To run the healthchecks for the "foo" and "bar" plugins
+                (assuming these plugins are on 'runtimepath' or 'packpath' and
+                they have implemented the Lua or Vimscript interface
+                require("foo.health").check() and health#bar#check(),
+                respectively): >
+                        :checkhealth foo bar
 <
-		To run healthchecks for lua submodules, use dot notation or
-		"*" to refer to all submodules. For example nvim provides
-		`vim.lsp` and `vim.treesitter`  >
-			:checkhealth vim.lsp vim.treesitter
-			:checkhealth vim*
+                To run healthchecks for lua submodules, use dot notation or
+                "*" to refer to all submodules. For example nvim provides
+                `vim.lsp` and `vim.treesitter`  >
+                        :checkhealth vim.lsp vim.treesitter
+                        :checkhealth vim*
 <
 ==============================================================================
-Lua Functions				*health-functions-lua* *health-lua*
+Lua Functions                           *health-functions-lua* *health-lua*
 
 The Lua "health" module can be used to create new healthchecks (see also
 |health-functions-vim|). To get started, simply use:  >
-	local health = require('health')
+        local health = require('health')
 <
-health.report_start({name})				*health.report_start()*
-	Starts a new report. Most plugins should call this only once, but if
-	you want different sections to appear in your report, call this once
-	per section.
+health.report_start({name})                             *health.report_start()*
+        Starts a new report. Most plugins should call this only once, but if
+        you want different sections to appear in your report, call this once
+        per section.
 
-health.report_info({msg})				*health.report_info()*
-	Reports an informational message.
+health.report_info({msg})                               *health.report_info()*
+        Reports an informational message.
 
-health.report_ok({msg})					*health.report_ok()*
-	Reports a "success" message.
+health.report_ok({msg})                                 *health.report_ok()*
+        Reports a "success" message.
 
-health.report_warn({msg} [, {advice}])			*health.report_warn()*
-	Reports a warning. {advice} is an optional List of suggestions.
+health.report_warn({msg} [, {advice}])                  *health.report_warn()*
+        Reports a warning. {advice} is an optional List of suggestions.
 
-health.report_error({msg} [, {advice}])			*health.report_error()*
-	Reports an error. {advice} is an optional List of suggestions.
+health.report_error({msg} [, {advice}])                 *health.report_error()*
+        Reports an error. {advice} is an optional List of suggestions.
 
 ==============================================================================
-Create a Lua healthcheck		*health-dev-lua*
+Create a Lua healthcheck                *health-dev-lua*
 
 Healthchecks are functions that check the user environment, configuration,
 etc. Nvim has built-in healthchecks in $VIMRUNTIME/autoload/health/.
@@ -83,71 +83,71 @@ will automatically find and invoke this function.
 
 If your plugin is named "foo", then its healthcheck module should be a file in
 one of these locations on 'runtimepath' or 'packpath':
-	- `lua/foo/health/init.lua`
-	- `lua/foo/health.lua`
+        - lua/foo/health/init.lua
+        - lua/foo/health.lua
 
 If your plugin provides a submodule named "bar" for which you want a separate
 healthcheck, define the healthcheck at one of these locations on 'runtimepath'
 or 'packpath':
-	- `lua/foo/bar/health/init.lua`
-	- `lua/foo/bar/health.lua`
+        - lua/foo/bar/health/init.lua
+        - lua/foo/bar/health.lua
 
 All submodules should return a Lua table containing the method `check()`.
 
 Copy this sample code into `lua/foo/health/init.lua` or `lua/foo/health.lua`,
 replacing "foo" in the path with your plugin name: >
 
-	local M = {}
-	local health = require("health")
+        local M = {}
+        local health = require("health")
 
-	M.check = function()
-	  health.report_start("my_plugin report")
-	  -- make sure setup function parameters are ok
-	  if check_setup() then
-	    health.report_ok("Setup function is correct")
-	  else
-	    health.report_error("Setup function is incorrect")
-	  end
-	  -- do some more checking
-	  -- ...
-	end
+        M.check = function()
+          health.report_start("my_plugin report")
+          -- make sure setup function parameters are ok
+          if check_setup() then
+            health.report_ok("Setup function is correct")
+          else
+            health.report_error("Setup function is incorrect")
+          end
+          -- do some more checking
+          -- ...
+        end
 
-	return M
+        return M
 
 ==============================================================================
-Vimscript Functions		*health-functions-vimscript* *health-vimscript*
+Vimscript Functions             *health-functions-vimscript* *health-vimscript*
 
 health.vim functions are for creating new healthchecks. (See also
 |health-functions-lua|)
 
-health#report_start({name})				*health#report_start*
-	Starts a new report. Most plugins should call this only once, but if
-	you want different sections to appear in your report, call this once
-	per section.
+health#report_start({name})                             *health#report_start*
+        Starts a new report. Most plugins should call this only once, but if
+        you want different sections to appear in your report, call this once
+        per section.
 
-health#report_info({msg})				*health#report_info*
-	Reports an informational message.
+health#report_info({msg})                               *health#report_info*
+        Reports an informational message.
 
-health#report_ok({msg})					*health#report_ok*
-	Reports a "success" message.
+health#report_ok({msg})                                 *health#report_ok*
+        Reports a "success" message.
 
-health#report_warn({msg} [, {advice}])			*health#report_warn*
-	Reports a warning. {advice} is an optional List of suggestions.
+health#report_warn({msg} [, {advice}])                  *health#report_warn*
+        Reports a warning. {advice} is an optional List of suggestions.
 
-health#report_error({msg} [, {advice}])			*health#report_error*
-	Reports an error. {advice} is an optional List of suggestions.
+health#report_error({msg} [, {advice}])                 *health#report_error*
+        Reports an error. {advice} is an optional List of suggestions.
 
-health#{plugin}#check()					*health.user_checker*
-	Healthcheck function for {plugin}. Called by |:checkhealth|
-	automatically. Example: >
+health#{plugin}#check()                                 *health.user_checker*
+        Healthcheck function for {plugin}. Called by |:checkhealth|
+        automatically. Example: >
 
-	function! health#my_plug#check() abort
-	  silent call s:check_environment_vars()
-	  silent call s:check_python_configuration()
-	endfunction
+        function! health#my_plug#check() abort
+          silent call s:check_environment_vars()
+          silent call s:check_python_configuration()
+        endfunction
 <
 ==============================================================================
-Create a healthcheck			*health-dev-vim*
+Create a healthcheck                    *health-dev-vim*
 
 Healthchecks are functions that check the user environment, configuration,
 etc. Nvim has built-in healthchecks in $VIMRUNTIME/autoload/health/.
@@ -157,24 +157,24 @@ health#{plugin}#check() function in autoload/health/{plugin}.vim.
 |:checkhealth| automatically finds and invokes such functions.
 
 If your plugin is named "foo", then its healthcheck function must be >
-	health#foo#check()
+        health#foo#check()
 
-defined in this file on 'runtimepath' or 'packpath': >
-	autoload/health/foo.vim
+defined in this file on 'runtimepath' or 'packpath':
+        - autoload/health/foo.vim
 
 Copy this sample code into autoload/health/foo.vim and replace "foo" with your
 plugin name: >
-	function! health#foo#check() abort
-	  call health#report_start('sanity checks')
-	  " perform arbitrary checks
-	  " ...
-	
-	  if looks_good
-	    call health#report_ok('found required dependencies')
-	  else
-	    call health#report_error('cannot find foo', 
-	      \ ['npm install --save foo'])
-	  endif
-	endfunction
-
-vim:noet tw=78:ts=8:ft=help:fdm=marker
+        function! health#foo#check() abort
+          call health#report_start('sanity checks')
+          " perform arbitrary checks
+          " ...
+        
+          if looks_good
+            call health#report_ok('found required dependencies')
+          else
+            call health#report_error('cannot find foo', 
+              \ ['npm install --save foo'])
+          endif
+        endfunction
+
+vim:et:tw=78:ts=8:ft=help:fdm=marker
diff --git a/test/functional/fixtures/lua/test_plug/autoload/health/test_plug.vim b/test/functional/fixtures/lua/test_plug/autoload/health/test_plug.vim
@@ -0,0 +1,3 @@
+function! health#success1#check()
+  call health#report_start("If you see this I'm broken")
+endfunction
diff --git a/test/functional/plugin/health_spec.lua b/test/functional/plugin/health_spec.lua
@@ -100,8 +100,10 @@ describe('health.vim', function()
         ]])
     end)
 
-    it("lua plugins", function()
+    it("lua plugins, skips vimscript healthchecks with the same name", function()
       command("checkhealth test_plug")
+      -- Existing file in test/functional/fixtures/lua/test_plug/autoload/health/test_plug.vim
+      -- and the Lua healthcheck is used instead.
       helpers.expect([[
 
         test_plug: require("test_plug.health").check()

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+function! health#success1#check()`
	`2`	`+ call health#report_start("If you see this I'm broken")`
	`3`	`+endfunction`