mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-15 07:36:50 +03:00
Fix scoped waivers in hooks and pattern scans
AI-assisted repairs requested by pbakaus. Resolve proposed HTML scopes against the original file; stamp browser CSS-pattern waivers across all active matches; combine attribute/config coverage in static HTML; reserve the visual budget for unwaived candidates while retaining a bounded waived sample; omit waived page-banner findings. Added failing-before Rust regressions and verified real Chrome direct/snapshot behavior. Workspace tests, bundle build, final release build, source distribution build, and rebuilt-engine full Bun/Node suite pass. Final live E2E sweep is running; the prior sweep passed 38 tests with one skip.
This commit is contained in:
@@ -135,7 +135,7 @@ if (IS_BROWSER && !__impeccable) {
|
||||
return {
|
||||
groupMap,
|
||||
allFindings: browserFindingsFromMap(groupMap),
|
||||
pageLevelFindings: collected.pageLevel,
|
||||
pageLevelFindings: collected.pageLevel.filter(f => !f.ignoredBy),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -473,7 +473,7 @@ if (IS_BROWSER && !__impeccable) {
|
||||
__impeccable.snapshot_clear();
|
||||
return {
|
||||
findings: serialized,
|
||||
pageLevel: out.pageLevel,
|
||||
pageLevel: out.pageLevel.filter(f => !f.ignoredBy),
|
||||
stats: { ...cap.stats, rounds, unknownStyleProps, captureMs: t1 - t0, coreMs: performance.now() - t1 },
|
||||
};
|
||||
};
|
||||
|
||||
@@ -155,7 +155,7 @@
|
||||
await ask(session, {
|
||||
stage: 'findings',
|
||||
groups,
|
||||
pageLevel: collected.pageLevel,
|
||||
pageLevel: collected.pageLevel.filter(f => !f.ignoredBy),
|
||||
serialized: serialize(wasm, groups),
|
||||
stats,
|
||||
});
|
||||
|
||||
@@ -763,6 +763,10 @@ pub fn selector_nodes_for_live_dom(dom: &dyn Dom, selector: &str) -> Option<Vec<
|
||||
/// pulsing-dot hero promotion. Returns `{ type, detail, severity? }`; the
|
||||
/// caller applies `_ruleOk`.
|
||||
pub fn scoped_html_pattern_findings(dom: &dyn Dom) -> Vec<BrowserFinding> {
|
||||
scoped_html_pattern_findings_with_ignores(dom, &[])
|
||||
}
|
||||
|
||||
fn scoped_html_pattern_findings_with_ignores(dom: &dyn Dom, ignores: &[SelectorIgnore]) -> Vec<BrowserFinding> {
|
||||
let html = dom.document_html_for_patterns();
|
||||
// Linked stylesheets are absent from the page's outerHTML, so the probe
|
||||
// hands their readable, live-resolving rules to the style corpus (#709).
|
||||
@@ -775,6 +779,7 @@ pub fn scoped_html_pattern_findings(dom: &dyn Dom) -> Vec<BrowserFinding> {
|
||||
let all = crate::checks::html_patterns::check_html_patterns(&html, Some(&corpora));
|
||||
let mut out = Vec::new();
|
||||
for f in all {
|
||||
let mut pattern_waived = None;
|
||||
if let Some(selector) = f.selector.as_deref().filter(|s| !s.is_empty()) {
|
||||
let Some(matches) = selector_nodes_for_live_dom(dom, selector) else {
|
||||
continue;
|
||||
@@ -782,11 +787,21 @@ pub fn scoped_html_pattern_findings(dom: &dyn Dom) -> Vec<BrowserFinding> {
|
||||
if matches.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if !matches.iter().any(|el| !scoped_ignore_active(dom, *el, &f.id)) {
|
||||
let active: Vec<_> = matches.into_iter().filter(|el| !scoped_ignore_active(dom, *el, &f.id)).collect();
|
||||
if active.is_empty() {
|
||||
continue;
|
||||
}
|
||||
// One CSS finding can cover many elements. Keep it reportable
|
||||
// unless every match not already attribute-waived is covered.
|
||||
for el in active {
|
||||
match waiving_selector(ignores, &f.id, |sel| matches!(dom.closest(el, sel), Ok(Some(_)))) {
|
||||
Some(sel) => { pattern_waived = pattern_waived.or(Some(sel.to_string())); }
|
||||
None => { pattern_waived = None; break; }
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut item = BrowserFinding::new(f.id.clone(), f.snippet.clone());
|
||||
item.ignored_by = pattern_waived;
|
||||
if let Some(sev) = f.severity.as_ref().filter(|s| !s.is_empty()) {
|
||||
item.severity = Some(sev.clone());
|
||||
} else if f.id == "pulsing-dot" {
|
||||
@@ -1475,7 +1490,7 @@ pub fn collect_browser_findings(dom: &dyn Dom, config: &BrowserConfig) -> Collec
|
||||
|
||||
page_pass(&mut groups, &mut page_level, q::check_page_quality_dom(dom));
|
||||
page_pass(&mut groups, &mut page_level, hits(pc::check_cream_palette(dom)));
|
||||
page_pass(&mut groups, &mut page_level, scoped_html_pattern_findings(dom));
|
||||
page_pass(&mut groups, &mut page_level, scoped_html_pattern_findings_with_ignores(dom, &config.ignore_selectors));
|
||||
|
||||
// Rule-pack page rules run after every built-in page pass, through the
|
||||
// same attribution as the built-in checks that name their own element.
|
||||
@@ -1917,6 +1932,30 @@ mod tests {
|
||||
assert_eq!(visual_contrast_result_el(&d, &pass), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pattern_selector_ignores_cover_matches_not_the_body() {
|
||||
let mut d = FakeDom::new();
|
||||
let (_, body) = d.with_page();
|
||||
d.html_for_patterns = "<style>.title { background: linear-gradient(90deg, #f00, #00f); -webkit-background-clip: text; color: transparent; }</style>".into();
|
||||
let a = d.add(Some(body), "h1");
|
||||
let b = d.add(Some(body), "h2");
|
||||
for el in [a, b] { d.add_selector(el, ".title"); }
|
||||
d.add_selector(a, ".Waived");
|
||||
let cfg = BrowserConfig {
|
||||
ignore_selectors: vec![SelectorIgnore::new("gradient-text", ".Waived")],
|
||||
..BrowserConfig::default()
|
||||
};
|
||||
let stamp = |d: &FakeDom| {
|
||||
collect_browser_findings(d, &cfg).groups.into_iter().flat_map(|g| g.findings)
|
||||
.find(|f| f.type_ == "gradient-text").expect("pattern finding").ignored_by
|
||||
};
|
||||
assert_eq!(stamp(&d), None, "one uncovered match keeps the finding");
|
||||
d.set_attr(b, "data-impeccable-ignore", "gradient-text");
|
||||
assert_eq!(stamp(&d).as_deref(), Some(".Waived"), "attribute and config coverage combine");
|
||||
d.add_selector(b, ".Waived");
|
||||
assert_eq!(stamp(&d).as_deref(), Some(".Waived"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_selector_ignores_stamp_the_component_and_its_subtree() {
|
||||
// The #34 shape: one component, many instances, one rule.
|
||||
|
||||
@@ -226,8 +226,9 @@ pub fn collect_visual_contrast_candidates_with_ignores(
|
||||
let body = dom.body();
|
||||
let root = dom.document_element();
|
||||
let mut candidates: Vec<Value> = Vec::new();
|
||||
let mut reportable_count = 0usize;
|
||||
for el in dom.query_all(None, "*").unwrap_or_default() {
|
||||
if (candidates.len() as f64) >= max_candidates {
|
||||
if (reportable_count as f64) >= max_candidates {
|
||||
break;
|
||||
}
|
||||
if closest_or_none(dom, el, OVERLAY_SELECTOR).is_some() {
|
||||
@@ -310,7 +311,14 @@ pub fn collect_visual_contrast_candidates_with_ignores(
|
||||
if let Some(selector) = impeccable_foundation::selector_ignores::waiving_selector(
|
||||
ignores, "low-contrast", |selector| matches!(dom.closest(el, selector), Ok(Some(_))),
|
||||
) {
|
||||
// Retain a bounded waived sample for suppression tallies without
|
||||
// spending the budget reserved for reportable candidates.
|
||||
if ((candidates.len() - reportable_count) as f64) >= max_candidates {
|
||||
continue;
|
||||
}
|
||||
m.insert("ignoredBy".into(), Value::String(selector.to_string()));
|
||||
} else {
|
||||
reportable_count += 1;
|
||||
}
|
||||
m.insert("tagName".into(), Value::String(tag));
|
||||
m.insert("text".into(), Value::String(text));
|
||||
@@ -1082,7 +1090,7 @@ mod tests {
|
||||
fn candidate_waivers_use_the_element_not_its_non_unique_display_selector() {
|
||||
let mut dom = FakeDom::new();
|
||||
let (_, body) = dom.with_page();
|
||||
for waived in [true, false] {
|
||||
for waived in [true, true, false] {
|
||||
let host = dom.add(Some(body), "section");
|
||||
dom.set_rect(host, 0.0, 0.0, 400.0, 300.0);
|
||||
if waived {
|
||||
@@ -1099,9 +1107,10 @@ mod tests {
|
||||
dom.set_styles(el, &[("color", "rgb(120, 120, 120)"), ("textShadow", "1px 1px black")]);
|
||||
}
|
||||
let candidates = collect_visual_contrast_candidates(&dom, &json!({
|
||||
"maxCandidates": 1,
|
||||
"ignoreSelectors": [{ "rule": "low-contrast", "selector": ".Waived" }]
|
||||
}));
|
||||
assert_eq!(candidates.len(), 2);
|
||||
assert_eq!(candidates.len(), 2, "one waived sample plus the reserved reportable slot");
|
||||
assert_eq!(candidates[0]["selector"], candidates[1]["selector"]);
|
||||
assert_eq!(candidates[0]["ignoredBy"], ".Waived");
|
||||
assert!(candidates[1].get("ignoredBy").is_none());
|
||||
|
||||
@@ -515,7 +515,11 @@ fn detect_proposed_html(
|
||||
let tmp = dir.join(jsp::basename(file_path));
|
||||
let result = (|| {
|
||||
std::fs::write(&tmp, content).map_err(|e| e.to_string())?;
|
||||
let findings = detector_detect_html(rt, &tmp.to_string_lossy(), scan)?;
|
||||
// Read proposed bytes from the temporary file, but resolve project
|
||||
// selector scopes against the file the user is actually editing.
|
||||
let findings = rt.html.detect_html(
|
||||
&tmp.to_string_lossy(), &scan.to_scan_options(file_path), &mut std::io::sink(),
|
||||
).map_err(|e| e.message)?;
|
||||
Ok(findings
|
||||
.into_iter()
|
||||
.map(|mut f| {
|
||||
@@ -916,3 +920,32 @@ pub fn run(rt: &Runtime, stdin: &str, io: &mut impeccable_common::Io) -> i32 {
|
||||
io.out(&out.stdout);
|
||||
0
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use impeccable_detect::engines::{EngineError, HtmlEngine, ScanOptions};
|
||||
|
||||
#[test]
|
||||
fn proposed_html_scopes_ignores_to_the_original_file() {
|
||||
struct Probe;
|
||||
impl HtmlEngine for Probe {
|
||||
fn detect_html(&self, path: &str, options: &ScanOptions, _: &mut dyn std::io::Write) -> Result<Vec<Finding>, EngineError> {
|
||||
assert!(path.contains("impeccable-pre-"), "content is read from the temporary file");
|
||||
assert_eq!(options.ignore_selectors.len(), 1, "scope uses the original target");
|
||||
assert_eq!(options.ignore_selectors[0].selector, ".Waived");
|
||||
Ok(vec![])
|
||||
}
|
||||
}
|
||||
let rt = Runtime::new("/project".into(), Default::default(), "/impeccable".into(), "/impeccable", &Probe);
|
||||
let scan = HookScanOptions {
|
||||
ignore_selectors: vec![impeccable_detect::config::IgnoreSelectorEntry {
|
||||
rule: "*".into(), selector: ".Waived".into(),
|
||||
files: Some(vec!["src/pages/**".into()]),
|
||||
..Default::default()
|
||||
}],
|
||||
..Default::default()
|
||||
};
|
||||
detect_proposed_html(&rt, "<h1>Proposed</h1>", "/project/src/pages/index.html", &scan).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -349,7 +349,7 @@ pub fn detect_html_source(
|
||||
// all-or-nothing rule the attribute pass above applies.
|
||||
if !matches.is_empty() {
|
||||
let mut covering: Option<&str> = None;
|
||||
for el in &matches {
|
||||
for el in matches.iter().filter(|el| !scoped_ignore_active(el, &f.id)) {
|
||||
match waiving_selector(options.ignore_selectors, &f.id, |sel| {
|
||||
el.closest(sel).is_some()
|
||||
}) {
|
||||
|
||||
@@ -41,6 +41,23 @@ fn undersized(findings: &[Finding]) -> Vec<&Finding> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pattern_waivers_combine_attribute_and_config_coverage() {
|
||||
let html = r#"<html><head><style>.title { background: linear-gradient(90deg, #f00, #00f); -webkit-background-clip: text; color: transparent; }</style></head><body>
|
||||
<h1 class="title Waived">One</h1><h2 class="title" data-impeccable-ignore="gradient-text">Two</h2>
|
||||
</body></html>"#;
|
||||
let entries = [SelectorIgnore::new("gradient-text", ".Waived")];
|
||||
let findings = scan(html, &entries);
|
||||
let hits: Vec<_> = findings.iter().filter(|f| f.antipattern == "gradient-text").collect();
|
||||
assert_eq!(hits.len(), 2, "one element finding and one stylesheet pattern");
|
||||
assert!(hits.iter().all(|f| ignored_by(f) == Some(".Waived")));
|
||||
let uncovered = scan(&html.replace("data-impeccable-ignore=\"gradient-text\"", ""), &entries);
|
||||
let hits: Vec<_> = uncovered.iter().filter(|f| f.antipattern == "gradient-text").collect();
|
||||
assert_eq!(hits.len(), 3, "two elements and one stylesheet pattern");
|
||||
assert_eq!(hits.iter().filter(|f| ignored_by(f).is_none()).count(), 2,
|
||||
"the uncovered element and shared pattern remain reportable");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn one_entry_covers_every_instance_of_the_component() {
|
||||
let before = scan(PAGE, &[]);
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user