Skip to content
Merged
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
109 changes: 48 additions & 61 deletions src/execute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,14 @@ fn merge_global_mappings_into_layers(specs: &mut [Plot], layer_schemas: &[Schema
let schema_columns: HashSet<&str> = schema.iter().map(|c| c.name.as_str()).collect();

// 1. First merge explicit global aesthetics (layer overrides global)
// Note: "color"/"colour" are accepted even though not in supported,
// because split_color_aesthetic will convert them to fill/stroke later
// Note: facet aesthetics (panel, row, column) are also accepted,
// as they apply to all layers regardless of geom support
// Note: Use all_names (not supported) so that Delayed aesthetics like
// pos2 on histogram can be targeted by explicit global mappings, matching
// the behavior of layer-level MAPPING
for (aesthetic, value) in &spec.global_mappings.aesthetics {
let is_color_alias = matches!(aesthetic.as_str(), "color" | "colour");
let is_facet_aesthetic = crate::plot::scale::is_facet_aesthetic(aesthetic.as_str());
if all_names.contains(&aesthetic.as_str()) || is_color_alias || is_facet_aesthetic {
if all_names.contains(&aesthetic.as_str()) || is_facet_aesthetic {
layer
.mappings
.aesthetics
Expand Down Expand Up @@ -226,69 +223,59 @@ fn merge_global_mappings_into_layers(specs: &mut [Plot], layer_schemas: &[Schema
}
}

/// Let 'color' aesthetics fill defaults for the 'stroke' and 'fill' aesthetics.
/// Also splits 'color' scale to 'fill' and 'stroke' scales.
/// Removes 'color' from both mappings and scales after splitting to avoid
/// non-deterministic behavior from HashMap iteration order.
fn split_color_aesthetic(spec: &mut Plot) {
// 1. Split color SCALE to fill/stroke scales
if let Some(color_scale_idx) = spec.scales.iter().position(|s| s.aesthetic == "color") {
let color_scale = spec.scales[color_scale_idx].clone();

// Add fill scale if not already present
if !spec.scales.iter().any(|s| s.aesthetic == "fill") {
let mut fill_scale = color_scale.clone();
fill_scale.aesthetic = "fill".to_string();
spec.scales.push(fill_scale);
}

// Add stroke scale if not already present
if !spec.scales.iter().any(|s| s.aesthetic == "stroke") {
let mut stroke_scale = color_scale.clone();
stroke_scale.aesthetic = "stroke".to_string();
spec.scales.push(stroke_scale);
}

// Remove the color scale
spec.scales.remove(color_scale_idx);
}

// 2. Split color mapping to fill/stroke in layers, then remove color
for layer in &mut spec.layers {
if let Some(color_value) = layer.mappings.aesthetics.get("color").cloned() {
let aesthetics = layer.geom.aesthetics();

for &aes in &["stroke", "fill"] {
if aesthetics.is_supported(aes) {
layer
.mappings
.aesthetics
.entry(aes.to_string())
.or_insert(color_value.clone());
/// Resolve aesthetic aliases in a plot specification.
///
/// For each alias defined in [`AESTHETIC_ALIASES`], splits the alias in scales,
/// layer mappings, and layer parameters into the concrete target aesthetics
/// (only where the geom supports them). Removes the alias after splitting to
/// avoid non-deterministic behavior from HashMap iteration order.
fn resolve_aesthetic_aliases(spec: &mut Plot) {
use crate::plot::layer::geom::types::AESTHETIC_ALIASES;

for &(alias, targets) in AESTHETIC_ALIASES {
// 1. Split alias SCALE to target scales
if let Some(idx) = spec.scales.iter().position(|s| s.aesthetic == alias) {
let alias_scale = spec.scales[idx].clone();
for &target in targets {
if !spec.scales.iter().any(|s| s.aesthetic == target) {
let mut target_scale = alias_scale.clone();
target_scale.aesthetic = target.to_string();
spec.scales.push(target_scale);
}
}

// Remove color after splitting
layer.mappings.aesthetics.remove("color");
spec.scales.remove(idx);
}
}

// 3. Split color parameter (SETTING) to fill/stroke in layers
for layer in &mut spec.layers {
if let Some(color_value) = layer.parameters.get("color").cloned() {
// 2. Split alias mapping and parameters in each layer
for layer in &mut spec.layers {
let aesthetics = layer.geom.aesthetics();

for &aes in &["stroke", "fill"] {
if aesthetics.is_supported(aes) {
layer
.parameters
.entry(aes.to_string())
.or_insert(color_value.clone());
// Split mapping
if let Some(value) = layer.mappings.aesthetics.get(alias).cloned() {
for &target in targets {
if aesthetics.is_supported(target) {
layer
.mappings
.aesthetics
.entry(target.to_string())
.or_insert(value.clone());
}
}
layer.mappings.aesthetics.remove(alias);
}

// Remove color after splitting
layer.parameters.remove("color");
// Split parameter (SETTING)
if let Some(value) = layer.parameters.get(alias).cloned() {
for &target in targets {
if aesthetics.is_supported(target) {
layer
.parameters
.entry(target.to_string())
.or_insert(value.clone());
}
}
layer.parameters.remove(alias);
}
}
}
}
Expand Down Expand Up @@ -1024,10 +1011,10 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result<Prep
// because transformation happens in builder.rs right after parsing
merge_global_mappings_into_layers(&mut specs, &layer_schemas);

// Split 'color' aesthetic to 'fill' and 'stroke' early in the pipeline
// This must happen before validation so fill/stroke are validated (not color)
// Resolve aesthetic aliases (e.g., 'color' 'fill'/'stroke') early in the pipeline
// This must happen before validation so concrete aesthetics are validated
for spec in &mut specs {
split_color_aesthetic(spec);
resolve_aesthetic_aliases(spec);
}

// Add literal (constant) columns to type info programmatically
Expand Down
97 changes: 86 additions & 11 deletions src/plot/layer/geom/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ pub const POSITION_VALUES: &[&str] = &["identity", "stack", "dodge", "jitter"];
/// Closed interval side values for binned data
pub const CLOSED_VALUES: &[&str] = &["left", "right"];

/// Aesthetic aliases: user-facing names that resolve to concrete aesthetics.
///
/// An alias is considered supported if any of its target aesthetics are supported
/// by a geom. For example, `color` resolves to `stroke` and/or `fill` — so any geom
/// that supports either `stroke` or `fill` also accepts `color`.
///
/// Note: Spelling variants (`colour`/`col` → `color`) are handled separately at parse
/// time by `normalise_aes_name()` in `parser/builder.rs`.
pub const AESTHETIC_ALIASES: &[(&str, &[&str])] = &[("color", &["stroke", "fill"])];

/// Default aesthetic values for a geom type
///
/// This struct describes which aesthetics a geom supports, requires, and their default values.
Expand All @@ -32,21 +42,37 @@ pub struct DefaultAesthetics {
}

impl DefaultAesthetics {
/// Get all aesthetic names (including Delayed)
/// Get all aesthetic names (including Delayed and aliases)
pub fn names(&self) -> Vec<&'static str> {
self.defaults.iter().map(|(name, _)| *name).collect()
let mut result: Vec<&'static str> = self.defaults.iter().map(|(name, _)| *name).collect();
// Include alias names if any of their targets are in the defaults
for &(alias, targets) in AESTHETIC_ALIASES {
if targets.iter().any(|t| result.contains(t)) {
result.push(alias);
}
}
result
}

/// Get supported aesthetic names (excludes Delayed, for MAPPING validation)
///
/// Returns the literal names from defaults. For bidirectional position checking,
/// use `is_supported()` which handles pos1/pos2 equivalence.
/// Returns the literal names from defaults plus any aliases whose targets are
/// supported. For bidirectional position checking, use `is_supported()` which
/// handles pos1/pos2 equivalence.
pub fn supported(&self) -> Vec<&'static str> {
self.defaults
let mut result: Vec<&'static str> = self
.defaults
.iter()
.filter(|(_, value)| !matches!(value, DefaultAestheticValue::Delayed))
.map(|(name, _)| *name)
.collect()
.collect();
// Include alias names if any of their targets are supported
for &(alias, targets) in AESTHETIC_ALIASES {
if targets.iter().any(|t| result.contains(t)) {
result.push(alias);
}
}
result
}

/// Get required aesthetic names (those marked as Required)
Expand All @@ -66,7 +92,8 @@ impl DefaultAesthetics {
/// Check if an aesthetic is supported (not Delayed)
///
/// Position aesthetics are bidirectional: if pos1* is supported, pos2* is also
/// considered supported (and vice versa).
/// considered supported (and vice versa). Aliases (e.g., `color`) are supported
/// if any of their target aesthetics are supported.
pub fn is_supported(&self, name: &str) -> bool {
// Check for direct match first
let direct_match = self
Expand All @@ -86,6 +113,13 @@ impl DefaultAesthetics {
});
}

// Check if name is an alias that resolves to a supported aesthetic
for &(alias, targets) in AESTHETIC_ALIASES {
if alias == name {
return targets.iter().any(|t| self.is_supported(t));
}
}

false
}

Expand Down Expand Up @@ -184,18 +218,20 @@ mod tests {
assert_eq!(aes.get("yend"), Some(&DefaultAestheticValue::Delayed));
assert_eq!(aes.get("nonexistent"), None);

// Test names() - includes all aesthetics
// Test names() - includes all aesthetics + aliases
let names = aes.names();
assert_eq!(names.len(), 6);
assert_eq!(names.len(), 7); // 6 defaults + "color" alias (has stroke+fill)
assert!(names.contains(&"x"));
assert!(names.contains(&"yend"));
assert!(names.contains(&"color")); // alias resolved from stroke+fill

// Test supported() - excludes Delayed
// Test supported() - excludes Delayed, includes aliases
let supported = aes.supported();
assert_eq!(supported.len(), 5);
assert_eq!(supported.len(), 6); // 5 non-delayed + "color" alias
assert!(supported.contains(&"x"));
assert!(supported.contains(&"size"));
assert!(supported.contains(&"fill"));
assert!(supported.contains(&"color")); // alias
assert!(!supported.contains(&"yend")); // Delayed excluded

// Test required() - only Required variants
Expand All @@ -208,6 +244,7 @@ mod tests {
// Test is_supported() - efficient membership check
assert!(aes.is_supported("x"));
assert!(aes.is_supported("size"));
assert!(aes.is_supported("color")); // alias: has stroke+fill
assert!(!aes.is_supported("yend")); // Delayed not supported
assert!(!aes.is_supported("nonexistent"));

Expand All @@ -222,4 +259,42 @@ mod tests {
assert!(!aes.is_required("size"));
assert!(!aes.is_required("yend"));
}

#[test]
fn test_color_alias_requires_stroke_or_fill() {
// Geom with neither stroke nor fill: color alias should NOT be supported
let aes = DefaultAesthetics {
defaults: &[
("pos1", DefaultAestheticValue::Required),
("pos2", DefaultAestheticValue::Required),
("size", DefaultAestheticValue::Number(3.0)),
],
};

assert!(!aes.is_supported("color"));
assert!(!aes.supported().contains(&"color"));
assert!(!aes.names().contains(&"color"));

// Geom with only stroke: color alias should be supported
let aes_stroke = DefaultAesthetics {
defaults: &[
("pos1", DefaultAestheticValue::Required),
("stroke", DefaultAestheticValue::String("black")),
],
};

assert!(aes_stroke.is_supported("color"));
assert!(aes_stroke.supported().contains(&"color"));

// Geom with only fill: color alias should be supported
let aes_fill = DefaultAesthetics {
defaults: &[
("pos1", DefaultAestheticValue::Required),
("fill", DefaultAestheticValue::String("black")),
],
};

assert!(aes_fill.is_supported("color"));
assert!(aes_fill.supported().contains(&"color"));
}
}
80 changes: 77 additions & 3 deletions src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,28 @@ pub fn validate(query: &str) -> Result<Validated> {

// Check required aesthetics
// Note: Without schema data, we can only check if mappings exist,
// not if the columns are valid. We skip this check for wildcards.
if !layer.mappings.wildcard {
if let Err(e) = layer.validate_mapping(&plot.aesthetic_context, false) {
// not if the columns are valid. We skip this check for wildcards
// (either layer or global).
let is_annotation = matches!(
layer.source,
Some(crate::plot::types::DataSource::Annotation)
);
let has_wildcard =
layer.mappings.wildcard || (!is_annotation && plot.global_mappings.wildcard);
if !has_wildcard {
// Merge global mappings into a temporary copy for validation
// (mirrors execution-time merge, layer takes precedence)
let mut merged = layer.clone();
if !is_annotation {
for (aesthetic, value) in &plot.global_mappings.aesthetics {
merged
.mappings
.aesthetics
.entry(aesthetic.clone())
.or_insert(value.clone());
}
}
if let Err(e) = merged.validate_mapping(&plot.aesthetic_context, false) {
errors.push(ValidationError {
message: format!("{}: {}", context, e),
location: None,
Expand Down Expand Up @@ -282,4 +301,59 @@ mod tests {
assert!(validated.valid());
assert!(validated.errors().is_empty());
}

#[test]
fn test_validate_color_aesthetic_on_line() {
// color should be valid on line geom (has stroke)
let validated = validate(
"SELECT 1 as x, 2 as y VISUALISE DRAW line MAPPING x AS x, y AS y, region AS color",
)
.unwrap();
assert!(
validated.valid(),
"color should be accepted on line geom: {:?}",
validated.errors()
);
}

#[test]
fn test_validate_color_aesthetic_on_point() {
// color should be valid on point geom (has stroke + fill)
let validated = validate(
"SELECT 1 as x, 2 as y VISUALISE DRAW point MAPPING x AS x, y AS y, cat AS color",
)
.unwrap();
assert!(
validated.valid(),
"color should be accepted on point geom: {:?}",
validated.errors()
);
}

#[test]
fn test_validate_colour_spelling() {
// British spelling 'colour' should work (normalized by parser to 'color')
let validated = validate(
"SELECT 1 as x, 2 as y VISUALISE DRAW line MAPPING x AS x, y AS y, region AS colour",
)
.unwrap();
assert!(
validated.valid(),
"colour (British) should be accepted: {:?}",
validated.errors()
);
}

#[test]
fn test_validate_global_color_mapping() {
// Global color mapping should validate correctly
let validated =
validate("SELECT 1 as x, 2 as y VISUALISE x AS x, y AS y, region AS color DRAW line")
.unwrap();
assert!(
validated.valid(),
"global color mapping should be accepted: {:?}",
validated.errors()
);
}
}
Loading