From 22fdec196d41a467347b8b613dcc9a8afaf3e128 Mon Sep 17 00:00:00 2001 From: cppbear Date: Wed, 7 Oct 2026 22:53:11 +0800 Subject: [PATCH] fix: handle existing coverage attributes and test-only helpers --- .github/workflows/ci.yml | 2 + docs/minimal-pipeline.md | 7 + docs/palm-rust-unit-test-generation.md | 6 +- scripts/check_coverage.py | 144 ++++++++++++++++++ utgen/README.md | 8 +- utgen/src/run/coverage.rs | 196 +++++++++++++++++++------ utgen/src/utils/preprocess.rs | 149 ++++++++++++++----- 7 files changed, 427 insertions(+), 85 deletions(-) create mode 100644 scripts/check_coverage.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04c43277f9..452fc50908 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,5 +38,7 @@ jobs: run: cargo metadata --manifest-path examples/bytes/Cargo.toml --no-deps --format-version 1 > /dev/null - name: Install the validated coverage tool run: cargo +stable install cargo-llvm-cov --version 0.6.16 --locked + - name: Check coverage annotation compatibility + run: python3 scripts/check_coverage.py - name: Validate the minimal analysis and test pipeline run: python3 scripts/check_minimal.py diff --git a/docs/minimal-pipeline.md b/docs/minimal-pipeline.md index d683a0d587..b1ff37e9cf 100644 --- a/docs/minimal-pipeline.md +++ b/docs/minimal-pipeline.md @@ -9,6 +9,7 @@ From the repository root: ```sh cargo build --workspace --locked cargo +stable install cargo-llvm-cov --version 0.6.16 --locked +python3 scripts/check_coverage.py python3 scripts/check_minimal.py ``` @@ -49,3 +50,9 @@ Recursive context lookup now resolves full function names through the compiler's The local macOS/Apple Silicon run passed using six generation responses and one repair response. Real-model behavior, bytes-scale execution, general multi-crate support, request concurrency limits, hard process timeouts, recovery after forced termination, and explicit run/resume behavior remain later work. Git history is retained. The coverage entry temporarily annotates sources with `#[coverage(off)]` and enables the nightly feature at crate roots without adding newlines. It preserves compiler flags, restores original bytes on returned errors, and performs no hashing or source fingerprint checks. `cargo llvm-cov report --json` reuses the first run's data; the first XML-producing command retains the tool's normal profile cleanup between candidates. This does not provide crash recovery or general macro expansion. See the [coverage command reference](../utgen/README.md#coverage-of-existing-tests) for scope. + +## Coverage Compatibility Regression + +`scripts/check_coverage.py` exercises 11 small standalone crates, without a model service or extra crate dependencies. It covers existing coverage attributes, `cfg_attr(test, ...)`, compound/nested conditions with Cargo features enabled and disabled, test-only methods and impls, file-level test modules, and an expression used by `include!`. Each case has an integration test so both normal-library and test-harness builds are checked. It compares execution outcomes, checks raw production-only coverage, verifies source restoration, and confirms preprocessing leaves no original tests while preserving byte offsets. A custom target entry containing an expression is rejected by both commands. + +The script runs in Linux CI. Coverage and preprocessing share only source parsing and test predicates; they keep separate edit visitors. Coverage attributes and their feature gate are added under the complement of existing conditions, without a cfg evaluator or compiler-flag overrides. Explicit `coverage(on)` remains an intentional opt-in. Expression fragments are preserved, without macro expansion or automatic test exclusion inside those fragments. diff --git a/docs/palm-rust-unit-test-generation.md b/docs/palm-rust-unit-test-generation.md index 93a06cbdfe..0a3145f40b 100644 --- a/docs/palm-rust-unit-test-generation.md +++ b/docs/palm-rust-unit-test-generation.md @@ -393,10 +393,10 @@ export PALM_CONFIG="$(pwd)/utgen/res/api.json" - `gen_test_rate` 读取生成结果，运行覆盖率，计算每个被测函数的测试数、编译通过数、运行通过数、行覆盖和分支覆盖。 - `utgen/src/run/coverage.rs` - - 覆盖率入口复用预处理的测试识别规则，在测试模块、独立测试函数及 `cfg(test)` 辅助函数前临时添加 `#[coverage(off)]`。整个测试模块（含生成的 `llmtests`）排除后，其中的辅助代码也不计入覆盖率分子和分母；被调用的生产函数仍参与统计。 - - 在 crate 根文件同一行临时开启 `coverage_attribute`，保持原行号，不覆盖编译 flags。成功或编译/导出报错返回前恢复源文件原始内容；强制终止后的恢复留待后续处理。 + - 覆盖率入口复用预处理的测试识别规则，在测试模块、独立测试函数及 `cfg(test)` 辅助函数/方法前临时添加 `#[coverage(off)]`。整个测试模块（含生成的 `llmtests`）排除后，其中的辅助代码默认不计入覆盖率分子和分母；测试专用 `impl` 按方法处理，带文件级 `#![cfg(test)]` 的模块整文件排除。被调用的生产函数仍参与统计。 + - 已有 coverage 属性和 feature 开关予以复用；条件属性只在原条件不成立时补充，条件由编译器判断，显式 `coverage(on)` 保留。在 crate 根文件同一行临时开启 `coverage_attribute`，保持原行号，不覆盖编译 flags。成功或编译/导出报错返回前恢复源文件原始内容；强制终止后的恢复留待后续处理。 - 测试只执行一次，先输出 XML，再由 `report --json` 导出同一份运行数据。第一条命令保留默认清理，避免候选之间累积旧覆盖率。 - - `utgen coverage -p <原始 crate 工作副本>` 可在预处理前测量原有测试，无需模型配置和分析产物；结果保留在该副本的 `coverage.xml`、`coverage.json`。断言失败仍导出覆盖率，编译或报告错误则返回失败。当前支持源码位于 `src/` 的独立 crate，不展开宏，也不自动推断未标记的公共辅助函数是否仅用于测试。 + - `utgen coverage -p <原始 crate 工作副本>` 可在预处理前测量原有测试，无需模型配置和分析产物；结果保留在该副本的 `coverage.xml`、`coverage.json`。断言失败仍导出覆盖率，编译或报告错误则返回失败。当前支持源码位于 `src/` 的独立 crate，非入口文件若是合法表达式片段（如 `include!` 引入的表达式），预处理与覆盖率均原样保留，不自动改写片段内部的测试代码；入口文件与其他解析错误仍严格检查。不展开宏，也不自动推断未标记的公共辅助函数是否仅用于测试。 - `utgen/src/run/coverage_json.rs` - 解析 `coverage.json` 中的 branch 信息。 diff --git a/scripts/check_coverage.py b/scripts/check_coverage.py new file mode 100644 index 0000000000..9a711b4638 --- /dev/null +++ b/scripts/check_coverage.py @@ -0,0 +1,144 @@ +#!/usr/bin/env python3 +"""Compile coverage compatibility cases in temporary standalone crates.""" +import argparse +import json +import os +from pathlib import Path +import re +import subprocess +import tempfile +import xml.etree.ElementTree as ET + +REPO = Path(__file__).resolve().parents[1] +PRODUCTION = 'pub fn production() -> u8 { 42 }\n' +TEST = '#[test] fn checks() { assert_eq!(production(), 42); }\n' +NIGHTLY_GATE = '#![cfg_attr(coverage_nightly, feature(coverage_attribute))]\n' +CONDITIONAL = { + 'src/lib.rs': ( + '#![cfg_attr(all(coverage_nightly, feature="existing"), feature(coverage_attribute))]\n' + + PRODUCTION + + '#[cfg_attr(coverage_nightly, cfg_attr(feature="existing", coverage(off)))]\n' + + TEST + ), +} +CASES = { + 'unconditional': { + 'src/lib.rs': '#![feature(coverage_attribute)]\n' + PRODUCTION + '#[coverage(off)]\n' + TEST, + }, + 'conditional_off': { + 'src/lib.rs': NIGHTLY_GATE + PRODUCTION + '#[cfg_attr(coverage_nightly, coverage(off))]\n' + TEST, + }, + 'test_gate': { + 'src/lib.rs': '#![cfg_attr(test, feature(coverage_attribute))]\n' + PRODUCTION + TEST, + }, + 'all_gate': { + 'src/lib.rs': '#![cfg_attr(all(coverage_nightly), feature(coverage_attribute))]\n' + PRODUCTION + TEST, + }, + 'feature_disabled': CONDITIONAL, + 'feature_enabled': CONDITIONAL, + 'method_cfg': { + 'src/lib.rs': PRODUCTION + 'struct S;\nimpl S { #[cfg(test)] fn method_helper() -> u8 { production() } }\n' + '#[test] fn checks() { assert_eq!(S::method_helper(), 42); }\n', + }, + 'impl_cfg': { + 'src/lib.rs': PRODUCTION + 'struct S;\n#[cfg(test)] impl S { fn impl_helper() -> u8 { production() } }\n' + '#[test] fn checks() { assert_eq!(S::impl_helper(), 42); }\n', + }, + 'inner_cfg': { + 'src/lib.rs': PRODUCTION + 'mod support;\n', + 'src/support.rs': '#![cfg(test)]\nfn inner_helper() -> u8 { super::production() }\n' + '#[test] fn checks() { assert_eq!(inner_helper(), 42); }\n', + }, + 'outer_cfg': { + 'src/lib.rs': PRODUCTION + '#[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod support;\n', + 'src/support.rs': 'fn outer_helper() -> u8 { super::production() }\n' + '#[test] fn checks() { assert_eq!(outer_helper(), 42); }\n', + }, + 'expression_fragment': { + 'src/lib.rs': 'pub fn production() -> u8 { include!("value.rs") }\n' + TEST, + 'src/value.rs': '42\n', + }, +} + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--bin-dir', type=Path, default=REPO / 'target/debug') + args = parser.parse_args() + utgen = str(args.bin_dir.resolve() / 'utgen') + work = Path(tempfile.mkdtemp(prefix='palm-coverage-check-')).resolve() + print(f'Coverage compatibility artifacts: {work}', flush=True) + env = os.environ.copy() + env.pop('CARGO_TARGET_DIR', None) + for name in ['PALM_CONFIG', 'PALM_API_BASE', 'PALM_API_KEY', 'PALM_MODEL']: + env.pop(name, None) + + def run(root, label, command, success=True): + result = subprocess.run(command, cwd=root, env=env, capture_output=True, text=True, timeout=180) + log = root / (label + '.log') + log.write_text(result.stdout + result.stderr) + assert (result.returncode == 0) == success, f'{label}: see {log}\n{(result.stdout + result.stderr)[-6000:]}' + return result.stdout + + for name, sources in CASES.items(): + print(name, flush=True) + root = work / name + root.mkdir() + default = '["existing"]' if name == 'feature_enabled' else '[]' + (root / 'Cargo.toml').write_text( + '[package]\nname="coverage_fixture"\nversion="0.1.0"\nedition="2021"\n' + f'[features]\nexisting=[]\ndefault={default}\n' + '[lints.rust]\nunexpected_cfgs={level="deny", check-cfg=["cfg(coverage_nightly)"]}\n' + ) + (root / 'rust-toolchain.toml').write_text('[toolchain]\nchannel="nightly-2025-03-19"\n') + # An integration test also compiles the library without cfg(test), so + # conditional feature gates must work in both compilation modes. + sources = sources | {'tests/integration.rs': + '#[test] fn integration() { assert_eq!(coverage_fixture::production(), 42); }\n'} + for relative, content in sources.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content) + baseline = {relative: (root / relative).read_bytes() for relative in sources} + original = run(root, 'ordinary-tests', ['cargo', 'test', '--offline', '--tests']) + covered = run(root, 'coverage', [utgen, 'coverage', '-p', str(root)]) + outcomes = r'test result: (\w+)\. (\d+) passed; (\d+) failed; (\d+) ignored;' + assert re.findall(outcomes, original) == re.findall(outcomes, covered) + assert all((root / relative).read_bytes() == contents for relative, contents in baseline.items()) + report = json.loads((root / 'coverage.json').read_text()) + functions = [f for data in report['data'] for f in data['functions']] + assert functions and all('production' in f['name'] for f in functions), functions + assert all(f['count'] > 0 for f in functions), functions + xml = ET.parse(root / 'coverage.xml') + assert all('helper' not in m.get('name', '') and 'checks' not in m.get('name', '') + for m in xml.iter('method')) + + run(root, 'preprocess', [utgen, 'pre-process', '-p', str(root)]) + for relative, original_bytes in baseline.items(): + if not relative.startswith('src/'): + continue + prepared = (root / relative).read_bytes() + assert len(prepared) == len(original_bytes) + assert [i for i, b in enumerate(prepared) if b == 10] == [i for i, b in enumerate(original_bytes) if b == 10] + if name == 'expression_fragment': + assert (root / 'src/value.rs').read_bytes() == baseline['src/value.rs'] + prepared_output = run(root, 'prepared-tests', ['cargo', 'test', '--offline', '--tests']) + assert re.findall(r'running (\d+) tests?', prepared_output) == ['0'], prepared_output + + # A custom Cargo entry path is still an entry, not an include! fragment. + root = work / 'invalid_entry' + (root / 'src').mkdir(parents=True) + (root / 'Cargo.toml').write_text( + '[package]\nname="invalid_entry"\nversion="0.1.0"\nedition="2021"\n' + '[lib]\npath="src/entry.rs"\n' + ) + (root / 'rust-toolchain.toml').write_text('[toolchain]\nchannel="nightly-2025-03-19"\n') + (root / 'src/entry.rs').write_text('42\n') + for command in ['coverage', 'pre-process']: + run(root, command, [utgen, command, '-p', str(root)], success=False) + assert (root / 'src/entry.rs').read_text() == '42\n' + print(f'All {len(CASES)} coverage/preprocessing cases passed.', flush=True) + + +if __name__ == '__main__': + main() diff --git a/utgen/README.md b/utgen/README.md index 7e92ae5260..6914462360 100644 --- a/utgen/README.md +++ b/utgen/README.md @@ -98,7 +98,7 @@ For multiple crates, use explicit work-directory paths: utgen pre-process -p -w -w ``` -The preprocessor removes modules whose `cfg` predicates are provably disabled without `test`, and functions with `test` or namespaced `test` attributes. It preserves production predicates such as `cfg(not(test))` and treats unknown feature/target predicates conservatively. Source files retain their line breaks and byte offsets. UTF-8 character columns can change inside blanked ranges; perform analysis after preprocessing. Existing `tests.bak` is never overwritten. There is no general reverse-preprocessing command. +The preprocessor removes modules whose `cfg` predicates are provably disabled without `test`, functions with `test` or namespaced `test` attributes, test-only impls/methods, and the contents of files with an inner `#![cfg(test)]` attribute. It preserves production predicates such as `cfg(not(test))` and treats unknown feature/target predicates conservatively. Source files retain their line breaks and byte offsets. UTF-8 character columns can change inside blanked ranges; perform analysis after preprocessing. Existing `tests.bak` is never overwritten. Non-entry files that parse as expressions (for example, `include!("value.rs")` fragments) are left unchanged; other parse failures remain errors. Cargo metadata identifies target entry files, which must parse as complete Rust files. There is no general reverse-preprocessing command. ### Analysis @@ -120,11 +120,11 @@ utgen coverage -p This command needs neither model configuration nor analysis artifacts. It writes `coverage.xml` and `coverage.json` in the crate directory from a single test run. Assertion failures remain visible in test output and coverage is still exported; compilation or report-export failures return a nonzero exit status. A failed command does not guarantee valid output files. -The same coverage entry point is used by generation and repair. It temporarily adds `#[coverage(off)]` to test-only modules (including generated `llmtests`), standalone test functions, and `#[cfg(test)]` helper functions. Module exclusion also covers nested helpers. Production functions called by tests remain instrumented; mixed production/test files are never excluded wholesale. Integration-test files use cargo-llvm-cov's default directory exclusions. Doc tests are not run by `--tests`. +The same coverage entry point is used by generation and repair. It temporarily adds `#[coverage(off)]` to test-only modules (including generated `llmtests`), standalone test functions, and `#[cfg(test)]` helper functions/methods. Methods inside test-only impls are excluded individually; files with `#![cfg(test)]` are excluded as a whole. Module exclusion also covers nested helpers. Production functions called by tests remain instrumented; mixed production/test files are never excluded wholesale. Integration-test files use cargo-llvm-cov's default directory exclusions. Doc tests are not run by `--tests`. -The nightly feature gate is inserted temporarily on the same line at crate roots, leaving compiler flags and Cargo configuration untouched. Ordinary compilation and repair need no coverage attributes. Coverage annotations are removed by restoring the bytes saved at coverage entry, after success or a returned build/report error. Temporary insertions preserve line numbers, but columns and byte offsets can change while measuring; use working copies and do not edit them concurrently. Recovery for the broader generation/repair backup flow and forced termination is still pending. +Existing coverage attributes and feature gates are reused. For conditional attributes, PALM adds a complementary `cfg_attr` only where the existing setting is absent; rustc evaluates the conditions for each build. Explicit `coverage(on)` settings are respected. The nightly feature gate is inserted temporarily on the same line at crate roots, leaving compiler flags and Cargo configuration untouched. Ordinary compilation and repair need no coverage attributes. Coverage annotations are removed by restoring the bytes saved at coverage entry, after success or a returned build/report error. Temporary insertions preserve line numbers, but columns and byte offsets can change while measuring; use working copies and do not edit them concurrently. Recovery for the broader generation/repair backup flow and forced termination is still pending. -Scope is a standalone crate with sources under `src/` and Cargo target entry files. Test identification reuses preprocessing's conservative `cfg` rules; macros are not expanded, and unmarked helpers outside test-only modules cannot be inferred as test-only. The pinned nightly and cargo-llvm-cov version are required. +Scope is a standalone crate with sources under `src/` and Cargo target entry files. Test identification reuses preprocessing's conservative `cfg` rules. Non-entry expression fragments are kept unchanged, including any code inside them; macros are not expanded, and unmarked helpers outside test-only modules cannot be inferred as test-only. The pinned nightly and cargo-llvm-cov version are required. ### Generation diff --git a/utgen/src/run/coverage.rs b/utgen/src/run/coverage.rs index 2de585c385..87a8391e82 100644 --- a/utgen/src/run/coverage.rs +++ b/utgen/src/run/coverage.rs @@ -1,56 +1,161 @@ -use crate::utils::test_ranges; +use crate::utils::{is_test_function, is_test_only, parse_source, source_span_offset}; use cargo_metadata::{MetadataCommand, TargetKind}; +use quote::quote; use std::{ collections::BTreeSet, fs, io, path::{Path, PathBuf}, process::{Command, Output}, }; -use syn::{Meta, Token, punctuated::Punctuated}; +use syn::{Meta, Token, punctuated::Punctuated, spanned::Spanned, visit::Visit}; use walkdir::WalkDir; -fn has_coverage_feature(meta: &Meta) -> bool { - let Meta::List(list) = meta else { return false }; +#[derive(Clone, Copy)] +enum CoverageAttribute { + Exclude, + Feature, +} + +// Collect the conditions under which an attribute already exists. Leave cfg +// evaluation to rustc, including test/non-test builds and Cargo feature flags. +fn attribute_conditions( + meta: &Meta, + kind: CoverageAttribute, + conditions: &mut Vec, + found: &mut Vec>, +) { + let Meta::List(list) = meta else { return }; let Ok(args) = list.parse_args_with(Punctuated::::parse_terminated) else { - return false; + return; }; - if list.path.is_ident("feature") { - args.iter() - .any(|arg| arg.path().is_ident("coverage_attribute")) - } else if list.path.is_ident("cfg_attr") - && args.first().is_some_and(|arg| { - arg.path().is_ident("coverage") || arg.path().is_ident("coverage_nightly") - }) - { - args.iter().skip(1).any(has_coverage_feature) + if list.path.is_ident("cfg_attr") { + let mut args = args.into_iter(); + if let Some(condition) = args.next() { + conditions.push(condition); + for attribute in args { + attribute_conditions(&attribute, kind, conditions, found); + } + conditions.pop(); + } } else { - false + let matches = match kind { + // Respect explicit coverage settings, including coverage(on). + CoverageAttribute::Exclude => list.path.is_ident("coverage"), + CoverageAttribute::Feature => { + list.path.is_ident("feature") + && args + .iter() + .any(|arg| arg.path().is_ident("coverage_attribute")) + } + }; + if matches { + found.push(conditions.clone()); + } + } +} + +fn missing_attribute(attrs: &[syn::Attribute], kind: CoverageAttribute, inner: bool) -> String { + let mut found = Vec::new(); + for attr in attrs { + attribute_conditions(&attr.meta, kind, &mut Vec::new(), &mut found); + } + if found.iter().any(Vec::is_empty) { + return String::new(); // Already unconditional. + } + let attribute = match kind { + CoverageAttribute::Exclude => quote!(coverage(off)), + CoverageAttribute::Feature => quote!(feature(coverage_attribute)), + }; + let attribute = if found.is_empty() { + attribute + } else { + let conditions = found.iter().map(|path| quote!(all(#(#path),*))); + quote!(cfg_attr(not(any(#(#conditions),*)), #attribute)) + }; + format!("#{}[{}] ", if inner { "!" } else { "" }, attribute) +} + +#[derive(Default)] +struct TestAnnotations { + insertions: Vec<(usize, String)>, + test_impl: bool, +} + +impl TestAnnotations { + fn exclude(&mut self, attrs: &[syn::Attribute], position: usize) { + let attribute = missing_attribute(attrs, CoverageAttribute::Exclude, false); + if !attribute.is_empty() { + self.insertions.push((position, attribute)); + } + } +} + +impl<'ast> Visit<'ast> for TestAnnotations { + fn visit_item_mod(&mut self, item: &'ast syn::ItemMod) { + if is_test_only(&item.attrs) { + self.exclude(&item.attrs, item.span().byte_range().start); + } else { + syn::visit::visit_item_mod(self, item); + } + } + fn visit_item_fn(&mut self, item: &'ast syn::ItemFn) { + if is_test_function(&item.attrs) { + self.exclude(&item.attrs, item.span().byte_range().start); + } else { + syn::visit::visit_item_fn(self, item); + } + } + fn visit_item_impl(&mut self, item: &'ast syn::ItemImpl) { + let previous = self.test_impl; + self.test_impl |= is_test_only(&item.attrs); + syn::visit::visit_item_impl(self, item); + self.test_impl = previous; + } + fn visit_impl_item_fn(&mut self, item: &'ast syn::ImplItemFn) { + if self.test_impl || is_test_function(&item.attrs) { + self.exclude(&item.attrs, item.span().byte_range().start); + } else { + syn::visit::visit_impl_item_fn(self, item); + } } } fn annotate_source(source: &str, crate_root: bool) -> syn::Result { + let Some(syntax) = parse_source(source, crate_root)? else { + return Ok(source.to_owned()); + }; let mut annotated = source.to_owned(); - // Reverse edits keep the original byte offsets valid. Inserting no newline - // preserves focal line ranges, including production following a same-line test. - for range in test_ranges(source)?.into_iter().rev() { - annotated.insert_str(range.start, "#[coverage(off)] "); - } + let mut file_attributes = String::new(); if crate_root { - let syntax = syn::parse_file(source)?; - if !syntax - .attrs - .iter() - .any(|attr| has_coverage_feature(&attr.meta)) - { - let bom = if source.starts_with('\u{feff}') { 3 } else { 0 }; - let offset = if syntax.shebang.is_some() { - source.find('\n').map_or(source.len(), |i| i + 1) - } else { - bom - }; - annotated.insert_str(offset, "#![feature(coverage_attribute)] "); + file_attributes.push_str(&missing_attribute( + &syntax.attrs, + CoverageAttribute::Feature, + true, + )); + } + if is_test_only(&syntax.attrs) { + file_attributes.push_str(&missing_attribute( + &syntax.attrs, + CoverageAttribute::Exclude, + true, + )); + } else { + let mut visitor = TestAnnotations::default(); + visitor.visit_file(&syntax); + let offset = source_span_offset(source, &syntax); + // Reverse edits keep original offsets valid; no newline is inserted. + for (position, attribute) in visitor.insertions.into_iter().rev() { + annotated.insert_str(position + offset, &attribute); } } + let prefix_end = if syntax.shebang.is_some() { + source.find('\n').map_or(source.len(), |i| i + 1) + } else if source.starts_with('\u{feff}') { + '\u{feff}'.len_utf8() + } else { + 0 + }; + annotated.insert_str(prefix_end, &file_attributes); Ok(annotated) } @@ -169,7 +274,7 @@ mod tests { fn test_modules_helpers_and_same_line_tests_keep_production_lines() { let source = "#[cfg(test)] mod tests { fn helper() {} #[test] fn a() {} }\r\n#[cfg(test)] fn helper() {}\r\n#[test] fn b() {} pub fn production() {}\r\n#[cfg(any(test, feature = \"shared\"))] mod shared {}\r\n"; let result = annotate_source(source, true).unwrap(); - assert_eq!(result.matches("#[coverage(off)]").count(), 3); + assert_eq!(result.matches("#[coverage (off)]").count(), 3); assert_eq!(result.matches('\r').count(), source.matches('\r').count()); assert_eq!(result.lines().count(), source.lines().count()); assert!( @@ -184,23 +289,28 @@ mod tests { } #[test] - fn existing_coverage_feature_is_not_duplicated() { - for gate in [ - "#![feature(coverage_attribute)]", - "#![cfg_attr(coverage_nightly, feature(coverage_attribute))]", - ] { - let source = format!("{gate}\n#[test] fn test() {{}}\n"); - let result = annotate_source(&source, true).unwrap(); - assert_eq!(result.matches("coverage_attribute").count(), 1); + fn unconditional_attributes_are_reused() { + for setting in ["off", "on"] { + let source = format!( + "#![feature(coverage_attribute)]\n#[cfg(test)] #[coverage({setting})] mod tests {{}}\n" + ); + assert_eq!(annotate_source(&source, true).unwrap(), source); } } + #[test] + fn expression_fragments_are_preserved_but_not_accepted_as_entry_files() { + assert_eq!(annotate_source("42\n", false).unwrap(), "42\n"); + assert!(annotate_source("42\n", true).is_err()); + assert!(annotate_source("fn broken( {", false).is_err()); + } + #[test] fn bom_and_shebang_precede_the_feature_gate() { let prefix = "\u{feff}#!/usr/bin/env rust-script\n"; let source = format!("{prefix}#[test] fn test() {{}} fn keep() {{}}\n"); let result = annotate_source(&source, true).unwrap(); - assert!(result.starts_with(&format!("{prefix}#![feature(coverage_attribute)]"))); + assert!(result.starts_with(&format!("{prefix}#![feature (coverage_attribute)]"))); assert_eq!(result.lines().count(), source.lines().count()); syn::parse_file(&result).unwrap(); } diff --git a/utgen/src/utils/preprocess.rs b/utgen/src/utils/preprocess.rs index 8e5bcd7bef..aacc8a0d71 100644 --- a/utgen/src/utils/preprocess.rs +++ b/utgen/src/utils/preprocess.rs @@ -1,4 +1,5 @@ -use std::{fs, io, ops::Range, path::Path}; +use cargo_metadata::MetadataCommand; +use std::{collections::BTreeSet, fs, io, ops::Range, path::Path}; use syn::{Meta, Token, punctuated::Punctuated, spanned::Spanned, visit::Visit}; use walkdir::WalkDir; @@ -26,6 +27,28 @@ pub fn comment_out_tests(dir: &Path) -> io::Result<()> { if !src.is_dir() { return Ok(()); } + let src = src.canonicalize()?; + // A Cargo entry must be a complete file. Non-entry expression fragments + // used by include! can be left alone. Directory-only preprocessing remains + // usable without requiring a manifest. + let roots: BTreeSet<_> = if dir.join("Cargo.toml").is_file() { + MetadataCommand::new() + .manifest_path(dir.join("Cargo.toml").canonicalize()?) + .current_dir(dir) + .no_deps() + .exec() + .map_err(io::Error::other)? + .packages + .into_iter() + .flat_map(|p| { + p.targets + .into_iter() + .map(|t| t.src_path.into_std_path_buf()) + }) + .collect() + } else { + BTreeSet::new() + }; // Parse every file before writing any of them. let mut changes = Vec::new(); for entry in WalkDir::new(src) { @@ -33,7 +56,7 @@ pub fn comment_out_tests(dir: &Path) -> io::Result<()> { let path = entry.path(); if entry.file_type().is_file() && path.extension().is_some_and(|e| e == "rs") { let original = fs::read_to_string(path)?; - let prepared = prepare_source(&original).map_err(|err| { + let prepared = prepare_source(&original, roots.contains(path)).map_err(|err| { io::Error::new( io::ErrorKind::InvalidData, format!("{}: {err}", path.display()), @@ -86,7 +109,7 @@ fn without_tests(meta: &Meta) -> Option { } } -fn is_test_module(attrs: &[syn::Attribute]) -> bool { +pub(crate) fn is_test_only(attrs: &[syn::Attribute]) -> bool { attrs.iter().any(|attr| { attr.path().is_ident("cfg") && attr @@ -96,6 +119,31 @@ fn is_test_module(attrs: &[syn::Attribute]) -> bool { }) } +pub(crate) fn is_test_function(attrs: &[syn::Attribute]) -> bool { + is_test_only(attrs) + || attrs + .iter() + .any(|a| a.path().segments.last().is_some_and(|s| s.ident == "test")) +} + +/// Expression fragments are deliberately not rewritten or macro-expanded. +pub(crate) fn parse_source(source: &str, crate_root: bool) -> syn::Result> { + match syn::parse_file(source) { + Ok(file) => Ok(Some(file)), + Err(_) if !crate_root && syn::parse_str::(source).is_ok() => Ok(None), + Err(error) => Err(error), + } +} + +pub(crate) fn source_span_offset(source: &str, syntax: &syn::File) -> usize { + // syn strips BOM/shebang prefixes before tokenizing. + (if source.starts_with('\u{feff}') { + '\u{feff}'.len_utf8() + } else { + 0 + }) + syntax.shebang.as_ref().map_or(0, |s| s.len()) +} + #[derive(Default)] struct TestRanges { ranges: Vec>, @@ -103,47 +151,53 @@ struct TestRanges { impl<'ast> Visit<'ast> for TestRanges { fn visit_item_mod(&mut self, item: &'ast syn::ItemMod) { - if is_test_module(&item.attrs) { + if is_test_only(&item.attrs) { self.ranges.push(item.span().byte_range()); } else { syn::visit::visit_item_mod(self, item); } } fn visit_item_fn(&mut self, item: &'ast syn::ItemFn) { - if is_test_module(&item.attrs) - || item - .attrs - .iter() - .any(|a| a.path().segments.last().is_some_and(|s| s.ident == "test")) - { + if is_test_function(&item.attrs) { self.ranges.push(item.span().byte_range()); } else { syn::visit::visit_item_fn(self, item); } } + fn visit_item_impl(&mut self, item: &'ast syn::ItemImpl) { + if is_test_only(&item.attrs) { + self.ranges.push(item.span().byte_range()); + } else { + syn::visit::visit_item_impl(self, item); + } + } + fn visit_impl_item_fn(&mut self, item: &'ast syn::ImplItemFn) { + if is_test_function(&item.attrs) { + self.ranges.push(item.span().byte_range()); + } else { + syn::visit::visit_impl_item_fn(self, item); + } + } } -pub(crate) fn test_ranges(source: &str) -> syn::Result>> { - let syntax = syn::parse_file(source)?; - // syn strips these prefixes before tokenizing; spans are relative to the - // remainder. Account for them when editing the original byte buffer. - let offset = if source.starts_with('\u{feff}') { - '\u{feff}'.len_utf8() - } else { - 0 - } + syntax.shebang.as_ref().map_or(0, |s| s.len()); +fn prepare_source(source: &str, crate_root: bool) -> syn::Result { + let Some(syntax) = parse_source(source, crate_root)? else { + return Ok(source.to_owned()); + }; + let offset = source_span_offset(source, &syntax); let mut visitor = TestRanges::default(); - visitor.visit_file(&syntax); - Ok(visitor - .ranges - .into_iter() - .map(|range| range.start + offset..range.end + offset) - .collect()) -} - -fn prepare_source(source: &str) -> syn::Result { + let ranges = if is_test_only(&syntax.attrs) { + vec![offset..source.len()] + } else { + visitor.visit_file(&syntax); + visitor + .ranges + .into_iter() + .map(|r| r.start + offset..r.end + offset) + .collect() + }; let mut bytes = source.as_bytes().to_vec(); - for range in test_ranges(source)? { + for range in ranges { // Retain newlines and byte offsets, including CRLF and same-line items. // Byte ranges include full UTF-8 characters; spaces keep valid UTF-8. for byte in &mut bytes[range] { @@ -162,19 +216,19 @@ mod tests { #[test] fn same_line_tests_do_not_move_production_code() { let source = "#[test] fn old() { let s = \"中文\"; } pub fn keep() -> u8 { 1 }\r\n"; - let prepared = prepare_source(source).unwrap(); + let prepared = prepare_source(source, false).unwrap(); assert_eq!(prepared.len(), source.len()); let position = source.find("pub fn keep").unwrap(); assert_eq!(&prepared[position..], &source[position..]); assert!(prepared[..position].trim().is_empty()); assert!(syn::parse_file(&prepared).is_ok()); - assert_eq!(prepare_source(&prepared).unwrap(), prepared); + assert_eq!(prepare_source(&prepared, false).unwrap(), prepared); } #[test] fn nested_ranges_leave_following_items_intact() { let source = "#[cfg(test)] mod tests { #[test] fn a() {} mod nested { #[test] fn b() {} } } fn keep() {}\n#[tokio::test] async fn c() {}\nfn last() {}\n"; - let prepared = prepare_source(source).unwrap(); + let prepared = prepare_source(source, false).unwrap(); assert_eq!(prepared.len(), source.len()); let syntax = syn::parse_file(&prepared).unwrap(); assert_eq!(syntax.items.len(), 2); @@ -185,7 +239,7 @@ mod tests { #[test] fn cfg_predicates_do_not_erase_production_modules() { let source = "#[cfg(not(test))] mod normal { fn a() {} }\n#[cfg(feature = \"contest\")] mod feature { fn b() {} }\n#[cfg(any(test, feature = \"enabled\"))] mod shared { fn c() {} }\n#[cfg(all(test, feature = \"enabled\"))] mod tests { fn d() {} }\n"; - let prepared = prepare_source(source).unwrap(); + let prepared = prepare_source(source, false).unwrap(); assert!(prepared.contains("mod normal")); assert!(prepared.contains("mod feature")); assert!(prepared.contains("mod shared")); @@ -196,7 +250,32 @@ mod tests { #[test] fn files_without_tests_are_byte_identical() { let source = "// 中文\r\npub fn keep() {}\r\n"; - assert_eq!(prepare_source(source).unwrap(), source); + assert_eq!(prepare_source(source, false).unwrap(), source); + } + + #[test] + fn test_methods_and_impls_are_removed_without_moving_production() { + let source = "struct S;\nimpl S { #[cfg(test)] fn helper() {} fn keep() {} }\n#[cfg(test)] impl S { fn other() {} }\nfn last() {}\n"; + let prepared = prepare_source(source, false).unwrap(); + assert!(!prepared.contains("helper")); + assert!(!prepared.contains("other")); + assert_eq!(prepared.len(), source.len()); + for name in ["fn keep", "fn last"] { + assert_eq!(prepared.find(name), source.find(name)); + } + syn::parse_file(&prepared).unwrap(); + } + + #[test] + fn file_level_tests_are_blanked_and_expression_fragments_preserved() { + let source = "#![cfg(test)]\r\nfn helper() {}\r\n#[test] fn checks() {}\r\n"; + let prepared = prepare_source(source, false).unwrap(); + assert!(prepared.trim().is_empty()); + assert_eq!(prepared.len(), source.len()); + assert_eq!(prepared.matches("\r\n").count(), 3); + assert_eq!(prepare_source("42\n", false).unwrap(), "42\n"); + assert!(prepare_source("42\n", true).is_err()); + assert!(prepare_source("fn broken( {", false).is_err()); } #[test] @@ -207,7 +286,7 @@ mod tests { "\u{feff}#!/usr/bin/env rust-script\n", ] { let source = format!("{prefix}#[test] fn old() {{}} fn keep() {{}}\n"); - let prepared = prepare_source(&source).unwrap(); + let prepared = prepare_source(&source, false).unwrap(); assert!(prepared.starts_with(prefix)); let position = source.find("fn keep").unwrap(); assert_eq!(&source[position..], &prepared[position..]);