Merge pull request #221204 from tweag/deprecate-paths-to-strings

lib.strings: Prevent paths as inputs in some functions

lib/strings.nix

@@ -2,7 +2,9 @@
 { lib }:
 let
 
-inherit (builtins) length;
+  inherit (builtins) length;
+
+  inherit (lib.trivial) warnIf;
 
 asciiTable = import ./ascii-table.nix;
 
@@ -207,7 +209,20 @@ rec {
        normalizePath "/a//b///c/"
        => "/a/b/c/"
   */
-  normalizePath = s: (builtins.foldl' (x: y: if y == "/" && hasSuffix "/" x then x else x+y) "" (stringToCharacters s));
+  normalizePath = s:
+    warnIf
+      (isPath s)
+      ''
+        lib.strings.normalizePath: The argument (${toString s}) is a path value, but only strings are supported.
+            Path values are always normalised in Nix, so there's no need to call this function on them.
+            This function also copies the path to the Nix store and returns the store path, the same as "''${path}" will, which may not be what you want.
+            This behavior is deprecated and will throw an error in the future.''
+      (
+        builtins.foldl'
+          (x: y: if y == "/" && hasSuffix "/" x then x else x+y)
+          ""
+          (stringToCharacters s)
+      );
 
   /* Depending on the boolean `cond', return either the given string
      or the empty string. Useful to concatenate against a bigger string.
@@ -240,7 +255,17 @@ rec {
     # Prefix to check for
     pref:
     # Input string
-    str: substring 0 (stringLength pref) str == pref;
+    str:
+    # Before 23.05, paths would be copied to the store before converting them
+    # to strings and comparing. This was surprising and confusing.
+    warnIf
+      (isPath pref)
+      ''
+        lib.strings.hasPrefix: The first argument (${toString pref}) is a path value, but only strings are supported.
+            There is almost certainly a bug in the calling code, since this function always returns `false` in such a case.
+            This function also copies the path to the Nix store, which may not be what you want.
+            This behavior is deprecated and will throw an error in the future.''
+      (substring 0 (stringLength pref) str == pref);
 
   /* Determine whether a string has given suffix.
 
@@ -260,8 +285,20 @@ rec {
     let
       lenContent = stringLength content;
       lenSuffix = stringLength suffix;
-    in lenContent >= lenSuffix &&
+    in
-       substring (lenContent - lenSuffix) lenContent content == suffix;
+    # Before 23.05, paths would be copied to the store before converting them
+    # to strings and comparing. This was surprising and confusing.
+    warnIf
+      (isPath suffix)
+      ''
+        lib.strings.hasSuffix: The first argument (${toString suffix}) is a path value, but only strings are supported.
+            There is almost certainly a bug in the calling code, since this function always returns `false` in such a case.
+            This function also copies the path to the Nix store, which may not be what you want.
+            This behavior is deprecated and will throw an error in the future.''
+      (
+        lenContent >= lenSuffix
+        && substring (lenContent - lenSuffix) lenContent content == suffix
+      );
 
   /* Determine whether a string contains the given infix
 
@@ -278,7 +315,16 @@ rec {
       => false
   */
   hasInfix = infix: content:
-    builtins.match ".*${escapeRegex infix}.*" "${content}" != null;
+    # Before 23.05, paths would be copied to the store before converting them
+    # to strings and comparing. This was surprising and confusing.
+    warnIf
+      (isPath infix)
+      ''
+        lib.strings.hasInfix: The first argument (${toString infix}) is a path value, but only strings are supported.
+            There is almost certainly a bug in the calling code, since this function always returns `false` in such a case.
+            This function also copies the path to the Nix store, which may not be what you want.
+            This behavior is deprecated and will throw an error in the future.''
+      (builtins.match ".*${escapeRegex infix}.*" "${content}" != null);
 
   /* Convert a string to a list of characters (i.e. singleton strings).
      This allows you to, e.g., map a function over each character.  However,
@@ -570,14 +616,23 @@ rec {
     prefix:
     # Input string
     str:
-    let
+    # Before 23.05, paths would be copied to the store before converting them
+    # to strings and comparing. This was surprising and confusing.
+    warnIf
+      (isPath prefix)
+      ''
+        lib.strings.removePrefix: The first argument (${toString prefix}) is a path value, but only strings are supported.
+            There is almost certainly a bug in the calling code, since this function never removes any prefix in such a case.
+            This function also copies the path to the Nix store, which may not be what you want.
+            This behavior is deprecated and will throw an error in the future.''
+    (let
       preLen = stringLength prefix;
       sLen = stringLength str;
     in
-      if hasPrefix prefix str then
+      if substring 0 preLen str == prefix then
         substring preLen (sLen - preLen) str
       else
-        str;
+        str);
 
   /* Return a string without the specified suffix, if the suffix matches.
 
@@ -594,14 +649,23 @@ rec {
     suffix:
     # Input string
     str:
-    let
+    # Before 23.05, paths would be copied to the store before converting them
+    # to strings and comparing. This was surprising and confusing.
+    warnIf
+      (isPath suffix)
+      ''
+        lib.strings.removeSuffix: The first argument (${toString suffix}) is a path value, but only strings are supported.
+            There is almost certainly a bug in the calling code, since this function never removes any suffix in such a case.
+            This function also copies the path to the Nix store, which may not be what you want.
+            This behavior is deprecated and will throw an error in the future.''
+    (let
       sufLen = stringLength suffix;
       sLen = stringLength str;
     in
       if sufLen <= sLen && suffix == substring (sLen - sufLen) sufLen str then
         substring 0 (sLen - sufLen) str
       else
-        str;
+        str);
 
   /* Return true if string v1 denotes a version older than v2.