Skip to content

Commit cec1614

Browse files
rizsottoclaude
andcommitted
docs(requirements): say what path formatting really reaches
The contract claimed that paths inside the arguments array are not transformed, and named output paths in flags as an example. Both halves were false: the source argument and the output flag's value have long been formatted with the same strategy as the file field, so an entry cannot disagree with itself about a path a consumer may read from either place. Only a path that is the payload of some other flag, an include search path and its kind, is carried through as the build spelled it. The code was right and the document was not, which is the dangerous way round for a contract that tests are written against: nothing pinned the real behaviour, so a later change could have "fixed" the converter to match the text and broken every consumer that resolves a source path from the arguments. The scenario is now written down and covered by a test that fails when the formatting is removed. The module doc carried the same claim, contradicting a comment ten lines into the function it describes, so it is corrected here too. Requirements: output-path-format Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 086816a commit cec1614

3 files changed

Lines changed: 94 additions & 7 deletions

File tree

crates/bear/src/output/clang/path_format.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@
2929
//! that fails falls back to its original path (see `CommandConverter` in
3030
//! `converter.rs`).
3131
//!
32-
//! The `arguments` attribute carries compiler flags, some of which embed file
33-
//! paths. These are intentionally left untransformed: rewriting them would
34-
//! require a flag-aware path rewriter for every compiler, which is fragile and
35-
//! out of scope.
32+
//! The source and output paths that reappear in the `arguments` attribute go
33+
//! through the `file` resolver too, so an entry cannot disagree with itself
34+
//! about a path. Paths embedded inside other flags (`-I../include` and its
35+
//! kind) are left untransformed: rewriting them would require a flag-aware
36+
//! path rewriter for every compiler, which is fragile and out of scope.
3637
3738
use crate::config::PathResolver;
3839
use std::io;

docs/requirements/output-path-format.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ Bear provides configurable path formatting for the `directory`, `file`, and
2323
- The `file` field path format is configurable
2424
- The `output` field is formatted using the same strategy as `file`; on
2525
formatting failure, Bear falls back to the original unformatted path
26+
- The source and output paths that appear in the `arguments` array are
27+
formatted with the same strategy as the `file` field
2628
- Four formatting behaviors are supported:
2729
- preserve the path exactly as observed during interception, with no
2830
transformation applied (the default)
@@ -51,9 +53,8 @@ Bear provides configurable path formatting for the `directory`, `file`, and
5153

5254
## Known limitations
5355

54-
- Paths inside the `arguments` array (include paths, output paths in
55-
flags) are deliberately not transformed. Rewriting them would require
56-
a compiler-flag-aware path rewriter, which is complex and error-prone.
56+
- Paths embedded inside other flags, such as include search paths, are
57+
not transformed; they are written exactly as the build spelled them.
5758

5859
## Testing
5960

@@ -115,6 +116,15 @@ output is written:
115116
> and `output` falls back to the original unformatted path
116117
> `obj/main.o`, with a warning.
117118
119+
Given a build invoked from `/home/user/project` that compiles
120+
`src/main.c` to `obj/main.o` and passes the include path `-I../include`:
121+
122+
> When path format for file is `absolute`,
123+
> then the `arguments` array carries the source as
124+
> `/home/user/project/src/main.c` and the output as
125+
> `/home/user/project/obj/main.o`,
126+
> and `-I../include` is carried unchanged.
127+
118128
Given path format for directory set to `relative`:
119129

120130
> Then the `directory` field resolves to `.` (relative to itself),

tests/integration/tests/cases/semantic.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,6 +1437,82 @@ format:
14371437
Ok(())
14381438
}
14391439

1440+
// Requirements: output-path-format
1441+
//
1442+
// The configured `file` strategy reaches the source and the output path where
1443+
// they reappear in `arguments`, so an entry cannot disagree with itself about
1444+
// a path a consumer reads from either place. A path that is the payload of
1445+
// some other flag (`-I../include`) is not individually addressable and is
1446+
// carried through exactly as the build spelled it.
1447+
#[test]
1448+
fn configured_file_format_reaches_the_argument_paths_but_not_flag_payloads() -> Result<()> {
1449+
// arrange
1450+
let env = TestEnvironment::new("argument_path_format")?;
1451+
let temp_dir = env.test_dir().to_str().unwrap();
1452+
1453+
let event = json!({
1454+
"executable": "gcc",
1455+
"arguments": ["gcc", "-c", "-I../include", "src/main.c", "-o", "obj/main.o"],
1456+
"working_dir": temp_dir,
1457+
"environment": {}
1458+
});
1459+
1460+
let config = r#"
1461+
schema: "4.2"
1462+
1463+
format:
1464+
paths:
1465+
directory: absolute
1466+
file: absolute
1467+
entries:
1468+
use_array_format: true
1469+
include_output_field: true
1470+
"#;
1471+
env.create_source_files(&[
1472+
("events.json", &event.to_string()),
1473+
("src/main.c", "int main() { return 0; }"),
1474+
])?;
1475+
let config_path = env.test_dir().join("config.yaml");
1476+
std::fs::write(&config_path, config)?;
1477+
1478+
// act
1479+
env.run_bear_success(&[
1480+
"--config",
1481+
config_path.to_str().unwrap(),
1482+
"semantic",
1483+
"--input",
1484+
"events.json",
1485+
"--output",
1486+
"compile_commands.json",
1487+
])?;
1488+
1489+
// assert
1490+
let sut = env.load_compilation_database("compile_commands.json")?;
1491+
sut.assert_count(1)?;
1492+
1493+
// Spell the expectations the way the code does, with `Path::join` and never
1494+
// a literal separator, so the assertion holds on Windows too.
1495+
let source = env.test_dir().join("src").join("main.c");
1496+
let object = env.test_dir().join("obj").join("main.o");
1497+
1498+
sut.assert_contains(
1499+
&CompilationEntryMatcher::new()
1500+
.file(source.to_str().unwrap())
1501+
.directory(temp_dir)
1502+
.output(object.to_str().unwrap())
1503+
.arguments(vec![
1504+
"gcc".to_string(),
1505+
"-c".to_string(),
1506+
"-I../include".to_string(),
1507+
source.to_str().unwrap().to_string(),
1508+
"-o".to_string(),
1509+
object.to_str().unwrap().to_string(),
1510+
]),
1511+
)?;
1512+
1513+
Ok(())
1514+
}
1515+
14401516
// Requirements: output-header-entries
14411517
//
14421518
// A synthesized header entry clones the donor's arguments with the output

0 commit comments

Comments
 (0)