From e0fd7a7be8bbbbf0f3cb6dc38cea3b62963eb0c9 Mon Sep 17 00:00:00 2001
From: Edwin Kofler <edwin@kofler.dev>
Date: Sat, 28 Jan 2023 19:52:22 -0800
Subject: [PATCH] fix: enforce & use consistent function definitions (#1464)

---
 lib/utils.bash          |  2 +-
 scripts/checkstyle.py   | 34 +++++++++++++++++++++++++++++++++-
 test/utils.bats         |  2 +-
 test/where_command.bats |  4 ++--
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/lib/utils.bash b/lib/utils.bash
index cecfdec6..862c4449 100644
--- a/lib/utils.bash
+++ b/lib/utils.bash
@@ -776,7 +776,7 @@ with_shim_executable() {
 
     # This function does get invoked, but shellcheck sees it as unused code
     # shellcheck disable=SC2317
-    function run_within_env() {
+    run_within_env() {
       local path
       path=$(remove_path_from_path "$PATH" "$(asdf_data_dir)/shims")
 
diff --git a/scripts/checkstyle.py b/scripts/checkstyle.py
index 9ddb36dd..7c027147 100755
--- a/scripts/checkstyle.py
+++ b/scripts/checkstyle.py
@@ -3,7 +3,7 @@ import re
 import os
 import argparse
 from pathlib import Path
-from typing import List, Dict, Any # compat
+from typing import Callable, List, Dict, Any # compat
 
 # This file checks Bash and Shell scripts for violations not found with
 # shellcheck or existing methods. You can use it in several ways:
@@ -33,6 +33,7 @@ class c:
     RESET = '\033[0m'
     BOLD = '\033[1m'
     UNDERLINE = '\033[4m'
+    LINK: Callable[[str, str], str] = lambda href, text: f'\033]8;;{href}\a{text}\033]8;;\a'
 
 def utilGetStrs(line, m):
     return (
@@ -62,6 +63,23 @@ def noTestDoubleEqualsFixer(line: str, m) -> str:
 
     return f'{prestr}={poststr}'
 
+# Before: function fn() { ...
+# After: fn() { ...
+# ---
+# Before: function fn { ...
+# After fn() { ...
+def noFunctionKeywordFixer(line: str, m) -> str:
+    prestr, midstr, poststr = utilGetStrs(line, m)
+
+    midstr = midstr.strip()
+    midstr = midstr[len('function'):]
+    midstr = midstr.strip()
+
+    parenIdx = midstr.find('(')
+    if parenIdx != -1: midstr = midstr[:parenIdx]
+
+    return f'{prestr}{midstr}() {poststr}'
+
 def lintfile(filepath: Path, rules: List[Rule], options: Dict[str, Any]):
     content_arr = filepath.read_text().split('\n')
 
@@ -140,6 +158,20 @@ def main():
             ],
             'found': 0
         },
+        {
+            'name': 'no-function-keyword',
+            'regex': '^[ \\t]*(?P<match>function .*?(?:\\([ \\t]*\\))?[ \\t]*){',
+            'reason': 'Only allow functions declared like `fn_name() {{ :; }}` for consistency (see ' + c.LINK('https://www.shellcheck.net/wiki/SC2113', 'ShellCheck SC2113') + ')',
+            'fixerFn': noFunctionKeywordFixer,
+            'testPositiveMatches': [
+                'function fn() { :; }',
+                'function fn { :; }',
+            ],
+            'testNegativeMatches': [
+                'fn() { :; }',
+            ],
+            'found': 0
+        },
     ]
 
     parser = argparse.ArgumentParser()
diff --git a/test/utils.bats b/test/utils.bats
index 580d62bf..288a2e22 100644
--- a/test/utils.bats
+++ b/test/utils.bats
@@ -453,7 +453,7 @@ EOF
 
   message="callback invoked"
 
-  function callback() {
+  callback() {
     echo "$message"
   }
 
diff --git a/test/where_command.bats b/test/where_command.bats
index ddb2c32b..491f6c2d 100644
--- a/test/where_command.bats
+++ b/test/where_command.bats
@@ -2,7 +2,7 @@
 
 load test_helpers
 
-function setup() {
+setup() {
   setup_asdf_dir
   install_dummy_plugin
   install_dummy_version 1.0
@@ -10,7 +10,7 @@ function setup() {
   install_dummy_version ref-master
 }
 
-function teardown() {
+teardown() {
   clean_asdf_dir
 }