add `function --shadow-builtin` flag

It's currently too easy for someone to bork their shell by doing something
like `function test; return 0; end`. That's obviously a silly, contrived,
example but the point is that novice users who learn about functions are
prone to do something like that without realizing it will bork the shell. Even
expert users who know about the `test` builtin might forget that, say, `pwd`
is a builtin.

This change adds a `--shadow-builtin` flag that must be specified to
indicate you know what you're doing.

Fixes #3000

3 files changed, 17 insertions, 30 deletions

diff --git a/share/functions/__fish_config_interactive.fish b/share/functions/__fish_config_interactive.fish
index 7b306e3d..ec891ddd 100644
--- a/share/functions/__fish_config_interactive.fish
+++ b/share/functions/__fish_config_interactive.fish
@@ -4,9 +4,7 @@
 # This function is called by the __fish_on_interactive function, which is defined in config.fish.
 #
 function __fish_config_interactive -d "Initializations that should be performed when entering interactive mode"
-
-
-    # Make sure this function is only run once
+    # Make sure this function is only run once.
     if set -q __fish_config_interactive_done
         return
     end
@@ -27,7 +25,6 @@ function __fish_config_interactive -d "Initializations that should be performed
     #
     # If we are starting up for the first time, set various defaults
     #
-
     if not set -q __fish_init_1_50_0
         if not set -q fish_greeting
             set -l line1 (printf (_ 'Welcome to fish, the friendly interactive shell') )
@@ -97,25 +94,22 @@ function __fish_config_interactive -d "Initializations that should be performed
         #
         # Directory history colors
         #
-
         set -q fish_color_history_current
         or set -U fish_color_history_current cyan
     end
 
     #
-    # Generate man page completions if not present
+    # Generate man page completions if not present.
     #
-
     if not test -d $userdatadir/fish/generated_completions
         #fish_update_completions is a function, so it can not be directly run in background.
         eval "$__fish_bin_dir/fish -c 'fish_update_completions > /dev/null ^/dev/null' &"
     end
 
     #
-    # Print a greeting
-    # fish_greeting can be a function (preferred) or a variable
+    # Print a greeting.
+    # fish_greeting can be a function (preferred) or a variable.
     #
-
     if functions -q fish_greeting
         fish_greeting
     else
@@ -130,7 +124,6 @@ function __fish_config_interactive -d "Initializations that should be performed
     # fish_color_cwd changes value. Like all event handlers, it can't be
     # autoloaded.
     #
-
     function __fish_repaint --on-variable fish_color_cwd --description "Event handler, repaints the prompt when fish_color_cwd changes"
         if status --is-interactive
             set -e __fish_prompt_cwd
@@ -149,7 +142,6 @@ function __fish_config_interactive -d "Initializations that should be performed
     # Completions for SysV startup scripts. These aren't bound to any
     # specific command, so they can't be autoloaded.
     #
-
     complete -x -p "/etc/init.d/*" -a start --description 'Start service'
     complete -x -p "/etc/init.d/*" -a stop --description 'Stop service'
     complete -x -p "/etc/init.d/*" -a status --description 'Print service status'
@@ -293,7 +285,7 @@ function __fish_config_interactive -d "Initializations that should be performed
 
         # Don't allow setting color other than what linux offers (see #2001)
         functions -e set_color
-        function set_color
+        function set_color --shadow-builtin
             set -l term_colors black red green yellow blue magenta cyan white normal
             for a in $argv
                 if not contains -- $a $term_colors
diff --git a/share/functions/cd.fish b/share/functions/cd.fish
index adf0327c..fdc8b607 100644
--- a/share/functions/cd.fish
+++ b/share/functions/cd.fish
@@ -1,7 +1,7 @@
 #
 # Wrap the builtin cd command to maintain directory history.
 #
-function cd --description "Change directory"
+function cd --shadow-builtin --description "Change directory"
     set -l MAX_DIR_HIST 25
 
     if test (count $argv) -gt 1
diff --git a/share/functions/history.fish b/share/functions/history.fish
index 0de39e30..a365ae19 100644
--- a/share/functions/history.fish
+++ b/share/functions/history.fish
@@ -1,16 +1,12 @@
 #
-#Deletes an item from history
+# Wrap the builtin history command to provide additional functionality.
 #
-function history --description "Deletes an item from history"
-
+function history --shadow-builtin --description "Deletes an item from history"
     set -l argc (count $argv)
     set -l prefix_args ""
     set -l contains_args ""
-
     set -l cmd print
-
     set -l search_mode none
-
     set -l pager less
     if set -q PAGER
         set pager $PAGER
@@ -46,7 +42,7 @@ function history --description "Deletes an item from history"
             end
         end
     else
-        #Execute history builtin without any argument
+        # Execute history builtin without any argument.
         if status --is-interactive
             builtin history | eval $pager
         else
@@ -57,9 +53,8 @@ function history --description "Deletes an item from history"
 
     switch $cmd
         case print
-            # Print matching items
-            # Note this may end up passing --search twice to the builtin,
-            # but that's harmless
+            # Print matching items. Note this may end up passing --search twice to the builtin,
+            # but that's harmless.
             builtin history --search $argv
 
         case delete
@@ -72,8 +67,7 @@ function history --description "Deletes an item from history"
                     set found_items (builtin history --search --contains $contains_args)
                 case none
                     builtin history $argv
-
-                    #Save changes after deleting item
+                    # Save changes after deleting item.
                     builtin history --save
                     return 0
             end
@@ -98,7 +92,8 @@ function history --description "Deletes an item from history"
                         continue
                     end
 
-                    #Following two validations could be embedded with "and" but I find the syntax kind of weird.
+                    # Following two validations could be embedded with "and" but I find the syntax
+                    # kind of weird.
                     if not string match -qr '^[0-9]+$' $i
                         printf "Invalid input: %s\n" $i
                         continue
@@ -124,18 +119,18 @@ function history --description "Deletes an item from history"
 
                     end
                 end
-                #Save changes after deleting item(s)
+                # Save changes after deleting item(s).
                 builtin history --save
             end
         case save
-            #Save changes to history file
+            # Save changes to history file.
             builtin history $argv
         case merge
             builtin history --merge
         case help
             builtin history --help
         case clear
-            # Erase the entire history
+            # Erase the entire history.
             echo "Are you sure you want to clear history ? (y/n)"
             read ch
             if test $ch = "y"