Skip to content

Commit

Permalink
revset, templater: add deprecation warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
@yuja
yuja committed Sep 22, 2024
1 parent ca32cc2 commit 1dd88d7
Show file tree
Hide file tree
Showing 4 changed files with 259 additions and 4 deletions.
27 changes: 24 additions & 3 deletions cli/src/commit_templater.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,14 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
);
map.insert(
"bookmarks",
|language, _diagnostics, _build_ctx, self_property, function| {
|language, diagnostics, _build_ctx, self_property, function| {
if function.name != "bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(TemplateParseError::expression(
"branches() is deprecated; use bookmarks() instead",
function.name_span,
));
}
function.expect_no_arguments()?;
let index = language
.keyword_cache
Expand All @@ -618,7 +625,14 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
);
map.insert(
"local_bookmarks",
|language, _diagnostics, _build_ctx, self_property, function| {
|language, diagnostics, _build_ctx, self_property, function| {
if function.name != "local_bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(TemplateParseError::expression(
"local_branches() is deprecated; use local_bookmarks() instead",
function.name_span,
));
}
function.expect_no_arguments()?;
let index = language
.keyword_cache
Expand All @@ -637,7 +651,14 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
);
map.insert(
"remote_bookmarks",
|language, _diagnostics, _build_ctx, self_property, function| {
|language, diagnostics, _build_ctx, self_property, function| {
if function.name != "remote_bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(TemplateParseError::expression(
"remote_branches() is deprecated; use remote_bookmarks() instead",
function.name_span,
));
}
function.expect_no_arguments()?;
let index = language
.keyword_cache
Expand Down
96 changes: 96 additions & 0 deletions cli/tests/test_revset_output.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,66 @@ fn test_bad_function_call() {
"###);
}

#[test]
fn test_parse_warning() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&[
"log",
"-r",
"branches() | remote_branches() | tracked_remote_branches() | \
untracked_remote_branches()",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
Warning: In revset expression
--> 1:1
|
1 | branches() | remote_branches() | tracked_remote_branches() | untracked_remote_branches()
| ^------^
|
= branches() is deprecated; use bookmarks() instead
Warning: In revset expression
--> 1:14
|
1 | branches() | remote_branches() | tracked_remote_branches() | untracked_remote_branches()
| ^-------------^
|
= remote_branches() is deprecated; use remote_bookmarks() instead
Warning: In revset expression
--> 1:34
|
1 | branches() | remote_branches() | tracked_remote_branches() | untracked_remote_branches()
| ^---------------------^
|
= tracked_remote_branches() is deprecated; use tracked_remote_bookmarks() instead
Warning: In revset expression
--> 1:62
|
1 | branches() | remote_branches() | tracked_remote_branches() | untracked_remote_branches()
| ^-----------------------^
|
= untracked_remote_branches() is deprecated; use untracked_remote_bookmarks() instead
"#);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "file(foo, bar)"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
Warning: In revset expression
--> 1:6
|
1 | file(foo, bar)
| ^------^
|
= Multi-argument patterns syntax is deprecated; separate them with |
"#);
}

#[test]
fn test_function_name_hint() {
let test_env = TestEnvironment::default();
Expand Down Expand Up @@ -363,6 +423,7 @@ fn test_alias() {
'recurse2()' = 'recurse'
'identity(x)' = 'x'
'my_author(x)' = 'author(x)'
'deprecated()' = 'branches()'
"###,
);

Expand Down Expand Up @@ -458,6 +519,41 @@ fn test_alias() {
|
= Alias "recurse" expanded recursively
"#);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "deprecated()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
Warning: In revset expression
--> 1:1
|
1 | deprecated()
| ^----------^
|
= In alias "deprecated()"
--> 1:1
|
1 | branches()
| ^------^
|
= branches() is deprecated; use bookmarks() instead
"#);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "all:deprecated()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
Warning: In revset expression
--> 1:5
|
1 | all:deprecated()
| ^----------^
|
= In alias "deprecated()"
--> 1:1
|
1 | branches()
| ^------^
|
= branches() is deprecated; use bookmarks() instead
"#);
}

#[test]
Expand Down
102 changes: 102 additions & 0 deletions cli/tests/test_templater.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

use std::path::Path;

use indoc::indoc;

use crate::common::TestEnvironment;

#[test]
Expand Down Expand Up @@ -115,6 +117,64 @@ fn test_templater_parse_error() {
"#);
}

#[test]
fn test_template_parse_warning() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

let template = indoc! {r#"
separate(' ',
branches,
local_branches,
remote_branches,
self.contained_in('branches()'),
)
"#};
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r@", "-T", template]);
insta::assert_snapshot!(stdout, @r#"
@ false
│
~
"#);
insta::assert_snapshot!(stderr, @r#"
Warning: In template expression
--> 2:3
|
2 | branches,
| ^------^
|
= branches() is deprecated; use bookmarks() instead
Warning: In template expression
--> 3:3
|
3 | local_branches,
| ^------------^
|
= local_branches() is deprecated; use local_bookmarks() instead
Warning: In template expression
--> 4:3
|
4 | remote_branches,
| ^-------------^
|
= remote_branches() is deprecated; use remote_bookmarks() instead
Warning: In template expression
--> 5:21
|
5 | self.contained_in('branches()'),
| ^----------^
|
= In revset expression
--> 1:1
|
1 | branches()
| ^------^
|
= branches() is deprecated; use bookmarks() instead
"#);
}

#[test]
fn test_templater_upper_lower() {
let test_env = TestEnvironment::default();
Expand Down Expand Up @@ -148,6 +208,7 @@ fn test_templater_alias() {
'recurse2()' = 'recurse'
'identity(x)' = 'x'
'coalesce(x, y)' = 'if(x, x, y)'
'deprecated()' = 'branches ++ self.contained_in("branches()")'
'builtin_log_node' = '"#"'
'builtin_op_log_node' = '"#"'
"###,
Expand Down Expand Up @@ -312,6 +373,47 @@ fn test_templater_alias() {
|
= Expected expression of type "Integer", but actual type is "String"
"#);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r@", "-Tdeprecated()"]);
insta::assert_snapshot!(stdout, @r##"
# false
│
~
"##);
insta::assert_snapshot!(stderr, @r#"
Warning: In template expression
--> 1:1
|
1 | deprecated()
| ^----------^
|
= In alias "deprecated()"
--> 1:1
|
1 | branches ++ self.contained_in("branches()")
| ^------^
|
= branches() is deprecated; use bookmarks() instead
Warning: In template expression
--> 1:1
|
1 | deprecated()
| ^----------^
|
= In alias "deprecated()"
--> 1:31
|
1 | branches ++ self.contained_in("branches()")
| ^----------^
|
= In revset expression
--> 1:1
|
1 | branches()
| ^------^
|
= branches() is deprecated; use bookmarks() instead
"#);
}

#[test]
Expand Down
38 changes: 37 additions & 1 deletion lib/src/revset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,13 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
Ok(RevsetExpression::root())
});
map.insert("bookmarks", |diagnostics, function, _context| {
if function.name != "bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(RevsetParseError::expression(
"branches() is deprecated; use bookmarks() instead",
function.name_span,
));
}
let ([], [opt_arg]) = function.expect_arguments()?;
let pattern = if let Some(arg) = opt_arg {
expect_string_pattern(diagnostics, arg)?
Expand All @@ -659,17 +666,40 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
Ok(RevsetExpression::bookmarks(pattern))
});
map.insert("remote_bookmarks", |diagnostics, function, _context| {
if function.name != "remote_bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(RevsetParseError::expression(
"remote_branches() is deprecated; use remote_bookmarks() instead",
function.name_span,
));
}
parse_remote_bookmarks_arguments(diagnostics, function, None)
});
map.insert(
"tracked_remote_bookmarks",
|diagnostics, function, _context| {
if function.name != "tracked_remote_bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(RevsetParseError::expression(
"tracked_remote_branches() is deprecated; use tracked_remote_bookmarks() \
instead",
function.name_span,
));
}
parse_remote_bookmarks_arguments(diagnostics, function, Some(RemoteRefState::Tracking))
},
);
map.insert(
"untracked_remote_bookmarks",
|diagnostics, function, _context| {
if function.name != "untracked_remote_bookmarks" {
// TODO: Remove in jj 0.28+
diagnostics.add_warning(RevsetParseError::expression(
"untracked_remote_branches() is deprecated; use untracked_remote_bookmarks() \
instead",
function.name_span,
));
}
parse_remote_bookmarks_arguments(diagnostics, function, Some(RemoteRefState::New))
},
);
Expand Down Expand Up @@ -766,8 +796,14 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
function.args_span, // TODO: better to use name_span?
)
})?;
// TODO: emit deprecation warning if multiple arguments are passed
// TODO: Drop support for multiple arguments in jj 0.28+
let ([arg], args) = function.expect_some_arguments()?;
if !args.is_empty() {
diagnostics.add_warning(RevsetParseError::expression(
"Multi-argument patterns syntax is deprecated; separate them with |",
function.args_span,
));
}
let file_expressions = itertools::chain([arg], args)
.map(|arg| expect_fileset_expression(diagnostics, arg, ctx.path_converter))
.try_collect()?;
Expand Down

0 comments on commit 1dd88d7

Please sign in to comment.