Skip to content

Improve error output when encountering git conflict markers #115413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 31 additions & 18 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2705,13 +2705,13 @@ impl<'a> Parser<'a> {
Ok(())
}

pub fn is_diff_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> bool {
pub fn is_conflict_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> bool {
(0..3).all(|i| self.look_ahead(i, |tok| tok == long_kind))
&& self.look_ahead(3, |tok| tok == short_kind)
}

fn diff_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> Option<Span> {
if self.is_diff_marker(long_kind, short_kind) {
fn conflict_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> Option<Span> {
if self.is_conflict_marker(long_kind, short_kind) {
let lo = self.token.span;
for _ in 0..4 {
self.bump();
Expand All @@ -2721,8 +2721,9 @@ impl<'a> Parser<'a> {
None
}

pub fn recover_diff_marker(&mut self) {
let Some(start) = self.diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) else {
pub fn recover_conflict_marker(&mut self) {
let Some(start) = self.conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt)
else {
return;
};
let mut spans = Vec::with_capacity(3);
Expand All @@ -2734,40 +2735,52 @@ impl<'a> Parser<'a> {
if self.token.kind == TokenKind::Eof {
break;
}
if let Some(span) = self.diff_marker(&TokenKind::OrOr, &TokenKind::BinOp(token::Or)) {
if let Some(span) = self.conflict_marker(&TokenKind::OrOr, &TokenKind::BinOp(token::Or))
{
middlediff3 = Some(span);
}
if let Some(span) = self.diff_marker(&TokenKind::EqEq, &TokenKind::Eq) {
if let Some(span) = self.conflict_marker(&TokenKind::EqEq, &TokenKind::Eq) {
middle = Some(span);
}
if let Some(span) = self.diff_marker(&TokenKind::BinOp(token::Shr), &TokenKind::Gt) {
if let Some(span) = self.conflict_marker(&TokenKind::BinOp(token::Shr), &TokenKind::Gt)
{
spans.push(span);
end = Some(span);
break;
}
self.bump();
}
let mut err = self.struct_span_err(spans, "encountered diff marker");
err.span_label(start, "after this is the code before the merge");
let mut err = self.struct_span_err(spans, "encountered git conflict marker");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git is not the only application that uses these kinds of conflict markers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that's true. What about the "for more information, visit the git documentation" note at the end, should that perhaps be reworded to something more along the lines of "if you're using git check out this documentation"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me (although I feel that git's docs are useful even if you're not using git specifically).


// with diff3
if let Some(middle) = middlediff3 {
err.span_label(middle, "");
err.span_label(
start,
"between this marker and `|||||||` is the code that we're merging into",
);
err.span_label(middle, "between this marker and `=======` is the base code (what the two refs diverged from)");
} else {
err.span_label(
start,
"between this marker and `=======` is the code that we're merging into",
);
}

if let Some(middle) = middle {
err.span_label(middle, "");
err.span_label(middle, "between this marker and `>>>>>>>` is the incoming code");
}

if let Some(end) = end {
err.span_label(end, "above this are the incoming code changes");
err.span_label(end, "");
}
err.help(
"if you're having merge conflicts after pulling new code, the top section is the code \
you already had and the bottom section is the remote code",
"conflict markers indicate that a merge was started but could not be completed due to merge conflicts",
);
err.help(
"if you're in the middle of a rebase, the top section is the code being rebased onto \
and the bottom section is the code coming from the current commit being rebased",
Comment on lines -2762 to -2767
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly concerned about removing these two notes, because I find them to be a significant part of what makes the error useful. Would you be open with adding them back in addition to the new ones? Maybe marking some as note (their title is a big less colorful).

"to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers",
);
err.note(
"for an explanation on these markers from the `git` documentation, visit \
"for more information, visit the `git` documentation \
<https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>",
);
err.emit();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3276,7 +3276,7 @@ impl<'a> Parser<'a> {
/// Parses `ident (COLON expr)?`.
fn parse_expr_field(&mut self) -> PResult<'a, ExprField> {
let attrs = self.parse_outer_attributes()?;
self.recover_diff_marker();
self.recover_conflict_marker();
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;

Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ impl<'a> Parser<'a> {
fn_parse_mode: FnParseMode,
force_collect: ForceCollect,
) -> PResult<'a, Option<Item>> {
self.recover_diff_marker();
self.recover_conflict_marker();
let attrs = self.parse_outer_attributes()?;
self.recover_diff_marker();
self.recover_conflict_marker();
self.parse_item_common(attrs, true, false, fn_parse_mode, force_collect)
}

Expand Down Expand Up @@ -685,7 +685,7 @@ impl<'a> Parser<'a> {
if self.recover_doc_comment_before_brace() {
continue;
}
self.recover_diff_marker();
self.recover_conflict_marker();
match parse_item(self) {
Ok(None) => {
let mut is_unnecessary_semicolon = !items.is_empty()
Expand Down Expand Up @@ -1014,7 +1014,7 @@ impl<'a> Parser<'a> {
/// ```
fn parse_use_tree_list(&mut self) -> PResult<'a, ThinVec<(UseTree, ast::NodeId)>> {
self.parse_delim_comma_seq(Delimiter::Brace, |p| {
p.recover_diff_marker();
p.recover_conflict_marker();
Ok((p.parse_use_tree()?, DUMMY_NODE_ID))
})
.map(|(r, _)| r)
Expand Down Expand Up @@ -1435,9 +1435,9 @@ impl<'a> Parser<'a> {
}

fn parse_enum_variant(&mut self) -> PResult<'a, Option<Variant>> {
self.recover_diff_marker();
self.recover_conflict_marker();
let variant_attrs = self.parse_outer_attributes()?;
self.recover_diff_marker();
self.recover_conflict_marker();
self.collect_tokens_trailing_token(
variant_attrs,
ForceCollect::No,
Expand Down Expand Up @@ -1647,8 +1647,8 @@ impl<'a> Parser<'a> {
let attrs = p.parse_outer_attributes()?;
p.collect_tokens_trailing_token(attrs, ForceCollect::No, |p, attrs| {
let mut snapshot = None;
if p.is_diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) {
// Account for `<<<<<<<` diff markers. We can't proactively error here because
if p.is_conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) {
// Account for `<<<<<<<` conflict markers. We can't proactively error here because
// that can be a valid type start, so we snapshot and reparse only we've
// encountered another parse error.
snapshot = Some(p.create_snapshot_for_diagnostic());
Expand All @@ -1658,7 +1658,7 @@ impl<'a> Parser<'a> {
Ok(vis) => vis,
Err(err) => {
if let Some(ref mut snapshot) = snapshot {
snapshot.recover_diff_marker();
snapshot.recover_conflict_marker();
}
return Err(err);
}
Expand All @@ -1667,7 +1667,7 @@ impl<'a> Parser<'a> {
Ok(ty) => ty,
Err(err) => {
if let Some(ref mut snapshot) = snapshot {
snapshot.recover_diff_marker();
snapshot.recover_conflict_marker();
}
return Err(err);
}
Expand All @@ -1692,9 +1692,9 @@ impl<'a> Parser<'a> {

/// Parses an element of a struct declaration.
fn parse_field_def(&mut self, adt_ty: &str) -> PResult<'a, FieldDef> {
self.recover_diff_marker();
self.recover_conflict_marker();
let attrs = self.parse_outer_attributes()?;
self.recover_diff_marker();
self.recover_conflict_marker();
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;
let vis = this.parse_visibility(FollowedByType::No)?;
Expand Down Expand Up @@ -2480,7 +2480,7 @@ impl<'a> Parser<'a> {
let mut first_param = true;
// Parse the arguments, starting out with `self` being allowed...
let (mut params, _) = self.parse_paren_comma_seq(|p| {
p.recover_diff_marker();
p.recover_conflict_marker();
let param = p.parse_param_general(req_name, first_param).or_else(|mut e| {
e.emit();
let lo = p.prev_token.span;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,16 +552,16 @@ impl<'a> Parser<'a> {
if self.token == token::Eof {
break;
}
if self.is_diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) {
// Account for `<<<<<<<` diff markers. We can't proactively error here because
if self.is_conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) {
// Account for `<<<<<<<` conflict markers. We can't proactively error here because
// that can be a valid path start, so we snapshot and reparse only we've
// encountered another parse error.
snapshot = Some(self.create_snapshot_for_diagnostic());
}
let stmt = match self.parse_full_stmt(recover) {
Err(mut err) if recover.yes() => {
if let Some(ref mut snapshot) = snapshot {
snapshot.recover_diff_marker();
snapshot.recover_conflict_marker();
}
if self.token == token::Colon {
// if next token is following a colon, it's likely a path
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/diff-markers/enum-2.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
enum E {
Foo {
<<<<<<< HEAD //~ ERROR encountered diff marker
<<<<<<< HEAD //~ ERROR encountered git conflict marker
x: u8,
|||||||
z: (),
Expand Down
16 changes: 8 additions & 8 deletions tests/ui/parser/diff-markers/enum-2.stderr
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
error: encountered diff marker
error: encountered git conflict marker
--> $DIR/enum-2.rs:3:1
|
LL | <<<<<<< HEAD
| ^^^^^^^ after this is the code before the merge
| ^^^^^^^ between this marker and `|||||||` is the code that we're merging into
LL | x: u8,
LL | |||||||
| -------
| ------- between this marker and `=======` is the base code (what the two refs diverged from)
LL | z: (),
LL | =======
| -------
| ------- between this marker and `>>>>>>>` is the incoming code
LL | y: i8,
LL | >>>>>>> branch
| ^^^^^^^ above this are the incoming code changes
| ^^^^^^^
|
= help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
= help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
= note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
= help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
= help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
= note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui/parser/diff-markers/enum.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
enum E {
<<<<<<< HEAD //~ ERROR encountered diff marker
<<<<<<< HEAD //~ ERROR encountered git conflict marker
Foo(u8),
=======
Bar(i8),
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/parser/diff-markers/enum.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
error: encountered diff marker
error: encountered git conflict marker
--> $DIR/enum.rs:2:1
|
LL | <<<<<<< HEAD
| ^^^^^^^ after this is the code before the merge
| ^^^^^^^ between this marker and `=======` is the code that we're merging into
LL | Foo(u8),
LL | =======
| -------
| ------- between this marker and `>>>>>>>` is the incoming code
LL | Bar(i8),
LL | >>>>>>> branch
| ^^^^^^^ above this are the incoming code changes
| ^^^^^^^
|
= help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
= help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
= note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
= help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
= help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
= note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui/parser/diff-markers/fn-arg.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
trait T {
fn foo(
<<<<<<< HEAD //~ ERROR encountered diff marker
<<<<<<< HEAD //~ ERROR encountered git conflict marker
x: u8,
=======
x: i8,
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/parser/diff-markers/fn-arg.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
error: encountered diff marker
error: encountered git conflict marker
--> $DIR/fn-arg.rs:3:1
|
LL | <<<<<<< HEAD
| ^^^^^^^ after this is the code before the merge
| ^^^^^^^ between this marker and `=======` is the code that we're merging into
LL | x: u8,
LL | =======
| -------
| ------- between this marker and `>>>>>>>` is the incoming code
LL | x: i8,
LL | >>>>>>> branch
| ^^^^^^^ above this are the incoming code changes
| ^^^^^^^
|
= help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
= help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
= note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
= help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
= help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
= note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui/parser/diff-markers/item-with-attr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#[attribute]
<<<<<<< HEAD //~ ERROR encountered diff marker
<<<<<<< HEAD //~ ERROR encountered git conflict marker
fn foo() {}
=======
fn bar() {}
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/parser/diff-markers/item-with-attr.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
error: encountered diff marker
error: encountered git conflict marker
--> $DIR/item-with-attr.rs:2:1
|
LL | <<<<<<< HEAD
| ^^^^^^^ after this is the code before the merge
| ^^^^^^^ between this marker and `=======` is the code that we're merging into
LL | fn foo() {}
LL | =======
| -------
| ------- between this marker and `>>>>>>>` is the incoming code
LL | fn bar() {}
LL | >>>>>>> branch
| ^^^^^^^ above this are the incoming code changes
| ^^^^^^^
|
= help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
= help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
= note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
= help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
= help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
= note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui/parser/diff-markers/item.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<<<<<<< HEAD //~ ERROR encountered diff marker
<<<<<<< HEAD //~ ERROR encountered git conflict marker
fn foo() {}
=======
fn bar() {}
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/parser/diff-markers/item.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
error: encountered diff marker
error: encountered git conflict marker
--> $DIR/item.rs:1:1
|
LL | <<<<<<< HEAD
| ^^^^^^^ after this is the code before the merge
| ^^^^^^^ between this marker and `=======` is the code that we're merging into
LL | fn foo() {}
LL | =======
| -------
| ------- between this marker and `>>>>>>>` is the incoming code
LL | fn bar() {}
LL | >>>>>>> branch
| ^^^^^^^ above this are the incoming code changes
| ^^^^^^^
|
= help: if you're having merge conflicts after pulling new code, the top section is the code you already had and the bottom section is the remote code
= help: if you're in the middle of a rebase, the top section is the code being rebased onto and the bottom section is the code coming from the current commit being rebased
= note: for an explanation on these markers from the `git` documentation, visit <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>
= help: conflict markers indicate that a merge was started but could not be completed due to merge conflicts
= help: to resolve a conflict, keep only the code you want and then delete the lines containing conflict markers
= note: for more information, visit the `git` documentation <https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging#_checking_out_conflicts>

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui/parser/diff-markers/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ struct S;
impl T for S {}

fn main() {
<<<<<<< HEAD //~ ERROR encountered diff marker
<<<<<<< HEAD //~ ERROR encountered git conflict marker
S::foo();
=======
S::bar();
Expand Down
Loading