Make indexing into `ColorScale`s safe (#3205)

Marshall Bowers created

This PR makes indexing into `ColorScale`s safe by constraining the
`ColorScaleStep`s to a set of known values.

Release Notes:

- N/A

Change summary

crates/storybook2/src/stories/colors.rs |  13 +
crates/theme2/src/default_colors.rs     |   2 
crates/theme2/src/scale.rs              | 162 ++++++++++++++++++++++++--
3 files changed, 155 insertions(+), 22 deletions(-)

Detailed changes

crates/storybook2/src/stories/colors.rs 🔗

@@ -1,6 +1,6 @@
 use crate::story::Story;
 use gpui2::{px, Div, Render};
-use theme2::default_color_scales;
+use theme2::{default_color_scales, ColorScaleStep};
 use ui::prelude::*;
 
 pub struct ColorsStory;
@@ -30,9 +30,14 @@ impl Render for ColorsStory {
                                     .line_height(px(24.))
                                     .child(scale.name().to_string()),
                             )
-                            .child(div().flex().gap_1().children(
-                                (1..=12).map(|step| div().flex().size_6().bg(scale.step(cx, step))),
-                            ))
+                            .child(
+                                div()
+                                    .flex()
+                                    .gap_1()
+                                    .children(ColorScaleStep::ALL.map(|step| {
+                                        div().flex().size_6().bg(scale.step(cx, step))
+                                    })),
+                            )
                     })),
             )
     }

crates/theme2/src/default_colors.rs 🔗

@@ -23,7 +23,7 @@ impl Default for SystemColors {
 impl Default for StatusColors {
     fn default() -> Self {
         Self {
-            conflict: gpui2::black(),
+            conflict: red().dark().step_11(),
             created: gpui2::black(),
             deleted: gpui2::black(),
             error: gpui2::black(),

crates/theme2/src/scale.rs 🔗

@@ -3,7 +3,62 @@ use gpui2::{AppContext, Hsla, SharedString};
 use crate::{ActiveTheme, Appearance};
 
 /// A one-based step in a [`ColorScale`].
-pub type ColorScaleStep = usize;
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)]
+pub struct ColorScaleStep(usize);
+
+impl ColorScaleStep {
+    /// The first step in a [`ColorScale`].
+    pub const ONE: Self = Self(1);
+
+    /// The second step in a [`ColorScale`].
+    pub const TWO: Self = Self(2);
+
+    /// The third step in a [`ColorScale`].
+    pub const THREE: Self = Self(3);
+
+    /// The fourth step in a [`ColorScale`].
+    pub const FOUR: Self = Self(4);
+
+    /// The fifth step in a [`ColorScale`].
+    pub const FIVE: Self = Self(5);
+
+    /// The sixth step in a [`ColorScale`].
+    pub const SIX: Self = Self(6);
+
+    /// The seventh step in a [`ColorScale`].
+    pub const SEVEN: Self = Self(7);
+
+    /// The eighth step in a [`ColorScale`].
+    pub const EIGHT: Self = Self(8);
+
+    /// The ninth step in a [`ColorScale`].
+    pub const NINE: Self = Self(9);
+
+    /// The tenth step in a [`ColorScale`].
+    pub const TEN: Self = Self(10);
+
+    /// The eleventh step in a [`ColorScale`].
+    pub const ELEVEN: Self = Self(11);
+
+    /// The twelfth step in a [`ColorScale`].
+    pub const TWELVE: Self = Self(12);
+
+    /// All of the steps in a [`ColorScale`].
+    pub const ALL: [ColorScaleStep; 12] = [
+        Self::ONE,
+        Self::TWO,
+        Self::THREE,
+        Self::FOUR,
+        Self::FIVE,
+        Self::SIX,
+        Self::SEVEN,
+        Self::EIGHT,
+        Self::NINE,
+        Self::TEN,
+        Self::ELEVEN,
+        Self::TWELVE,
+    ];
+}
 
 pub struct ColorScale(Vec<Hsla>);
 
@@ -13,11 +68,84 @@ impl FromIterator<Hsla> for ColorScale {
     }
 }
 
-impl std::ops::Index<ColorScaleStep> for ColorScale {
-    type Output = Hsla;
+impl ColorScale {
+    /// Returns the specified step in the [`ColorScale`].
+    #[inline]
+    pub fn step(&self, step: ColorScaleStep) -> Hsla {
+        // Steps are one-based, so we need convert to the zero-based vec index.
+        self.0[step.0 - 1]
+    }
+
+    /// Returns the first step in the [`ColorScale`].
+    #[inline]
+    pub fn step_1(&self) -> Hsla {
+        self.step(ColorScaleStep::ONE)
+    }
+
+    /// Returns the second step in the [`ColorScale`].
+    #[inline]
+    pub fn step_2(&self) -> Hsla {
+        self.step(ColorScaleStep::TWO)
+    }
+
+    /// Returns the third step in the [`ColorScale`].
+    #[inline]
+    pub fn step_3(&self) -> Hsla {
+        self.step(ColorScaleStep::THREE)
+    }
+
+    /// Returns the fourth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_4(&self) -> Hsla {
+        self.step(ColorScaleStep::FOUR)
+    }
+
+    /// Returns the fifth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_5(&self) -> Hsla {
+        self.step(ColorScaleStep::FIVE)
+    }
+
+    /// Returns the sixth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_6(&self) -> Hsla {
+        self.step(ColorScaleStep::SIX)
+    }
+
+    /// Returns the seventh step in the [`ColorScale`].
+    #[inline]
+    pub fn step_7(&self) -> Hsla {
+        self.step(ColorScaleStep::SEVEN)
+    }
+
+    /// Returns the eighth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_8(&self) -> Hsla {
+        self.step(ColorScaleStep::EIGHT)
+    }
+
+    /// Returns the ninth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_9(&self) -> Hsla {
+        self.step(ColorScaleStep::NINE)
+    }
+
+    /// Returns the tenth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_10(&self) -> Hsla {
+        self.step(ColorScaleStep::TEN)
+    }
+
+    /// Returns the eleventh step in the [`ColorScale`].
+    #[inline]
+    pub fn step_11(&self) -> Hsla {
+        self.step(ColorScaleStep::ELEVEN)
+    }
 
-    fn index(&self, index: ColorScaleStep) -> &Self::Output {
-        &self.0[index - 1]
+    /// Returns the twelfth step in the [`ColorScale`].
+    #[inline]
+    pub fn step_12(&self) -> Hsla {
+        self.step(ColorScaleStep::TWELVE)
     }
 }
 
@@ -131,33 +259,33 @@ impl ColorScaleSet {
         &self.name
     }
 
-    pub fn light(&self, step: ColorScaleStep) -> Hsla {
-        self.light[step - 1]
+    pub fn light(&self) -> &ColorScale {
+        &self.light
     }
 
-    pub fn light_alpha(&self, step: ColorScaleStep) -> Hsla {
-        self.light_alpha[step - 1]
+    pub fn light_alpha(&self) -> &ColorScale {
+        &self.light_alpha
     }
 
-    pub fn dark(&self, step: ColorScaleStep) -> Hsla {
-        self.dark[step - 1]
+    pub fn dark(&self) -> &ColorScale {
+        &self.dark
     }
 
-    pub fn dark_alpha(&self, step: ColorScaleStep) -> Hsla {
-        self.dark_alpha[step - 1]
+    pub fn dark_alpha(&self) -> &ColorScale {
+        &self.dark_alpha
     }
 
     pub fn step(&self, cx: &AppContext, step: ColorScaleStep) -> Hsla {
         match cx.theme().appearance {
-            Appearance::Light => self.light(step),
-            Appearance::Dark => self.dark(step),
+            Appearance::Light => self.light().step(step),
+            Appearance::Dark => self.dark().step(step),
         }
     }
 
     pub fn step_alpha(&self, cx: &AppContext, step: ColorScaleStep) -> Hsla {
         match cx.theme().appearance {
-            Appearance::Light => self.light_alpha(step),
-            Appearance::Dark => self.dark_alpha(step),
+            Appearance::Light => self.light_alpha.step(step),
+            Appearance::Dark => self.dark_alpha.step(step),
         }
     }
 }