Define builtin_immutable_heads() as a default revset alias.

* Add `builtin_immutable_heads()` in the `revsets.toml`.
* Redefine `immutable_heads()` in terms of `builtin_immutable_heads()`
* Warn if user redefines `builtin_immutable_heads()`, `mutable()` or
  `immutable()`.
* Update module constant in revset_util.rs from BUILTIN_IMMUTABLE_HEADS
  to USER_IMMUTABLE_HEADS to avoid confusion since it points at
  `immutable_heads()` **and** we now have a revset-alias
  literally named `builtin_immutable_heads()`.
* Add unittest
* Update CHANGELOG
* Update documentation.

Fixes: #4162

CHANGELOG.md

@@ -65,6 +65,12 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### New features
 
+* Define `immutable_heads()` revset alias in terms of a new `builtin_immutable_heads()`.
+  This enables users to redefine `immutable_heads()` as they wish, but still
+  have `builtin_immutable_heads()` which can should not be redefined. Also 
+  issue warnings if any of `mutable()`, `immutable()` or `builtin_immutable_heads()`
+  are redefined by the user.
+
 * External diff tools can now be configured to invoke the tool on each file
   individually instead of being passed a directory by setting
   `merge-tools.$TOOL.diff-invocation-mode="file-by-file"` in config.toml.

cli/src/config/revsets.toml

@@ -18,6 +18,7 @@ latest(
 )
 '''
 
-'immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()'
+'builtin_immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()'
+'immutable_heads()' = 'builtin_immutable_heads()'
 'immutable()' = '::(immutable_heads() | root())'
 'mutable()' = '~immutable()'

cli/src/revset_util.rs

@@ -31,12 +31,12 @@ use jj_lib::settings::ConfigResultExt as _;
 use thiserror::Error;
 
 use crate::command_error::{user_error, CommandError};
-use crate::config::LayeredConfigs;
+use crate::config::{ConfigSource, LayeredConfigs};
 use crate::formatter::Formatter;
 use crate::templater::TemplateRenderer;
 use crate::ui::Ui;
 
-const BUILTIN_IMMUTABLE_HEADS: &str = "immutable_heads";
+const USER_IMMUTABLE_HEADS: &str = "immutable_heads";
 
 #[derive(Debug, Error)]
 pub enum UserRevsetEvaluationError {
@@ -107,6 +107,31 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
     }
 }
 
+fn warn_user_redefined_builtin(
+    ui: &Ui,
+    source: &ConfigSource,
+    name: &str,
+) -> Result<(), CommandError> {
+    match source {
+        ConfigSource::Env | ConfigSource::User | ConfigSource::Repo | ConfigSource::CommandArg => {
+            let checked_mutability_builtins =
+                vec!["mutable()", "immutable()", "builtin_immutable_heads()"];
+
+            if checked_mutability_builtins.contains(&name) {
+                writeln!(
+                    ui.warning_default(),
+                    "redefining `revset-aliases.{}` is not recommended; redefine \
+                     `immutable_heads()` instead",
+                    name
+                )?;
+            }
+        }
+        ConfigSource::Default => (),
+    }
+
+    Ok(())
+}
+
 pub fn load_revset_aliases(
     ui: &Ui,
     layered_configs: &LayeredConfigs,
@@ -115,17 +140,20 @@ pub fn load_revset_aliases(
     let mut aliases_map = RevsetAliasesMap::new();
     // Load from all config layers in order. 'f(x)' in default layer should be
     // overridden by 'f(a)' in user.
-    for (_, config) in layered_configs.sources() {
+    for (source, config) in layered_configs.sources() {
         let table = if let Some(table) = config.get_table(TABLE_KEY).optional()? {
             table
         } else {
             continue;
         };
         for (decl, value) in table.into_iter().sorted_by(|a, b| a.0.cmp(&b.0)) {
+            warn_user_redefined_builtin(ui, &source, &decl)?;
+
             let r = value
                 .into_string()
                 .map_err(|e| e.to_string())
                 .and_then(|v| aliases_map.insert(&decl, v).map_err(|e| e.to_string()));
+
             if let Err(s) = r {
                 writeln!(
                     ui.warning_default(),
@@ -167,7 +195,7 @@ pub fn parse_immutable_heads_expression(
 ) -> Result<Rc<RevsetExpression>, RevsetParseError> {
     let (_, _, immutable_heads_str) = context
         .aliases_map()
-        .get_function(BUILTIN_IMMUTABLE_HEADS, 0)
+        .get_function(USER_IMMUTABLE_HEADS, 0)
         .unwrap();
     let heads = revset::parse(immutable_heads_str, context)?;
     Ok(heads.union(&RevsetExpression::root()))

cli/tests/test_builtin_aliases.rs

@@ -136,3 +136,24 @@ fn test_builtin_alias_trunk_no_match_only_exact() {
     insta::assert_snapshot!(stderr, @r###"
     "###);
 }
+
+#[test]
+fn test_builtin_user_redefines_builtin_immutable_heads() {
+    let (test_env, workspace_root) = set_up("main");
+
+    test_env.add_config(r#"revset-aliases.'builtin_immutable_heads()' = '@'"#);
+    test_env.add_config(r#"revset-aliases.'mutable()' = '@'"#);
+    test_env.add_config(r#"revset-aliases.'immutable()' = '@'"#);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-r", "trunk()"]);
+    insta::assert_snapshot!(stdout, @r###"
+    ○  xtvrqkyv [email protected] 2001-02-03 08:05:08 main d13ecdbd
+    │  (empty) description 1
+    ~
+    "###);
+    insta::assert_snapshot!(stderr, @r###"
+    Warning: redefining `revset-aliases.builtin_immutable_heads()` is not recommended; redefine `immutable_heads()` instead
+    Warning: redefining `revset-aliases.immutable()` is not recommended; redefine `immutable_heads()` instead
+    Warning: redefining `revset-aliases.mutable()` is not recommended; redefine `immutable_heads()` instead
+    "###);
+}

docs/revsets.md

@@ -426,19 +426,24 @@ for a comprehensive list.
   'trunk()' = 'your-branch@your-remote'
   ```
 
+* `builtin_immutable_heads()`: Resolves to `trunk() | tags() | untracked_remote_branches()`.
+   It is used as the default definition for `immutable_heads()` below. it is not
+   recommended to redefined this alias. Prefer to redefine `immutable_heads()` instead.
+
 * `immutable_heads()`: Resolves to `trunk() | tags() |
-  untracked_remote_branches()` by default. See
-  [here](config.md#set-of-immutable-commits) for details.
+  untracked_remote_branches()` by default. It is actually defined as `builtin_immutable_heads()`,
+  and can be overridden as required.
+  See [here](config.md#set-of-immutable-commits) for details.
 
 * `immutable()`: The set of commits that `jj` treats as immutable. This is
-  equivalent to `::(immutable_heads() | root())`. Note that modifying this will
-  *not* change whether a commit is immutable. To do that, edit
-  `immutable_heads()`.
+  equivalent to `::(immutable_heads() | root())`. It is not recommended to redefine
+  this alias. Note that modifying this will *not* change whether a commit is immutable.
+  To do that, edit `immutable_heads()`. 
 
 * `mutable()`: The set of commits that `jj` treats as mutable. This is
-  equivalent to `~immutable()`. Note that modifying this will
-  *not* change whether a commit is immutable. To do that, edit
-  `immutable_heads()`.
+  equivalent to `~immutable()`. It is not recommended to redefined this alias.
+  Note that modifying this will *not* change whether a commit is immutable.
+  To do that, edit `immutable_heads()`.
 
 
 ## The `all:` modifier