Use NonZeroU32 to ensure settings tabsize cannot be zero

Keith Simmons created

Change summary

crates/editor/src/display_map.rs           |  6 +-
crates/editor/src/display_map/block_map.rs |  9 ++--
crates/editor/src/display_map/tab_map.rs   | 46 ++++++++++++++++-------
crates/editor/src/display_map/wrap_map.rs  |  6 +-
crates/editor/src/editor.rs                | 21 ++++++----
crates/editor/src/multi_buffer.rs          |  4 +-
crates/project/src/project.rs              |  4 +-
crates/settings/src/settings.rs            |  8 ++--
crates/zed/src/main.rs                     | 14 +++---
crates/zed/src/settings_file.rs            | 20 +++++-----
crates/zed/src/zed.rs                      |  3 +
11 files changed, 83 insertions(+), 58 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -13,7 +13,7 @@ use gpui::{
 };
 use language::{Point, Subscription as BufferSubscription};
 use settings::Settings;
-use std::{any::TypeId, fmt::Debug, ops::Range, sync::Arc};
+use std::{any::TypeId, fmt::Debug, num::NonZeroU32, ops::Range, sync::Arc};
 use sum_tree::{Bias, TreeMap};
 use tab_map::TabMap;
 use wrap_map::WrapMap;
@@ -203,7 +203,7 @@ impl DisplayMap {
             .update(cx, |map, cx| map.set_wrap_width(width, cx))
     }
 
-    fn tab_size(buffer: &ModelHandle<MultiBuffer>, cx: &mut ModelContext<Self>) -> u32 {
+    fn tab_size(buffer: &ModelHandle<MultiBuffer>, cx: &mut ModelContext<Self>) -> NonZeroU32 {
         let language_name = buffer
             .read(cx)
             .as_singleton()
@@ -966,7 +966,7 @@ pub mod tests {
         language.set_theme(&theme);
         cx.update(|cx| {
             let mut settings = Settings::test(cx);
-            settings.language_settings.tab_size = Some(2);
+            settings.language_settings.tab_size = Some(2.try_into().unwrap());
             cx.set_global(settings);
         });
 

crates/editor/src/display_map/block_map.rs 🔗

@@ -1025,7 +1025,7 @@ mod tests {
         let buffer_snapshot = buffer.read(cx).snapshot(cx);
         let subscription = buffer.update(cx, |buffer, _| buffer.subscribe());
         let (fold_map, folds_snapshot) = FoldMap::new(buffer_snapshot.clone());
-        let (tab_map, tabs_snapshot) = TabMap::new(folds_snapshot.clone(), 1);
+        let (tab_map, tabs_snapshot) = TabMap::new(folds_snapshot.clone(), 1.try_into().unwrap());
         let (wrap_map, wraps_snapshot) = WrapMap::new(tabs_snapshot, font_id, 14.0, None, cx);
         let mut block_map = BlockMap::new(wraps_snapshot.clone(), 1, 1);
 
@@ -1170,7 +1170,8 @@ mod tests {
 
         let (folds_snapshot, fold_edits) =
             fold_map.read(buffer_snapshot, subscription.consume().into_inner());
-        let (tabs_snapshot, tab_edits) = tab_map.sync(folds_snapshot, fold_edits, 4);
+        let (tabs_snapshot, tab_edits) =
+            tab_map.sync(folds_snapshot, fold_edits, 4.try_into().unwrap());
         let (wraps_snapshot, wrap_edits) = wrap_map.update(cx, |wrap_map, cx| {
             wrap_map.sync(tabs_snapshot, tab_edits, cx)
         });
@@ -1193,7 +1194,7 @@ mod tests {
         let buffer = MultiBuffer::build_simple(text, cx);
         let buffer_snapshot = buffer.read(cx).snapshot(cx);
         let (_, folds_snapshot) = FoldMap::new(buffer_snapshot.clone());
-        let (_, tabs_snapshot) = TabMap::new(folds_snapshot.clone(), 1);
+        let (_, tabs_snapshot) = TabMap::new(folds_snapshot.clone(), 1.try_into().unwrap());
         let (_, wraps_snapshot) = WrapMap::new(tabs_snapshot, font_id, 14.0, Some(60.), cx);
         let mut block_map = BlockMap::new(wraps_snapshot.clone(), 1, 1);
 
@@ -1237,7 +1238,7 @@ mod tests {
         } else {
             Some(rng.gen_range(0.0..=100.0))
         };
-        let tab_size = 1;
+        let tab_size = 1.try_into().unwrap();
         let family_id = cx.font_cache().load_family(&["Helvetica"]).unwrap();
         let font_id = cx
             .font_cache()

crates/editor/src/display_map/tab_map.rs 🔗

@@ -5,14 +5,14 @@ use super::{
 use crate::MultiBufferSnapshot;
 use language::{rope, Chunk};
 use parking_lot::Mutex;
-use std::{cmp, mem, ops::Range};
+use std::{cmp, mem, num::NonZeroU32, ops::Range};
 use sum_tree::Bias;
 use text::Point;
 
 pub struct TabMap(Mutex<TabSnapshot>);
 
 impl TabMap {
-    pub fn new(input: FoldSnapshot, tab_size: u32) -> (Self, TabSnapshot) {
+    pub fn new(input: FoldSnapshot, tab_size: NonZeroU32) -> (Self, TabSnapshot) {
         let snapshot = TabSnapshot {
             fold_snapshot: input,
             tab_size,
@@ -24,7 +24,7 @@ impl TabMap {
         &self,
         fold_snapshot: FoldSnapshot,
         mut fold_edits: Vec<FoldEdit>,
-        tab_size: u32,
+        tab_size: NonZeroU32,
     ) -> (TabSnapshot, Vec<TabEdit>) {
         let mut old_snapshot = self.0.lock();
         let max_offset = old_snapshot.fold_snapshot.len();
@@ -88,7 +88,7 @@ impl TabMap {
 #[derive(Clone)]
 pub struct TabSnapshot {
     pub fold_snapshot: FoldSnapshot,
-    pub tab_size: u32,
+    pub tab_size: NonZeroU32,
 }
 
 impl TabSnapshot {
@@ -251,7 +251,11 @@ impl TabSnapshot {
             .to_buffer_point(&self.fold_snapshot)
     }
 
-    fn expand_tabs(chars: impl Iterator<Item = char>, column: usize, tab_size: u32) -> usize {
+    fn expand_tabs(
+        chars: impl Iterator<Item = char>,
+        column: usize,
+        tab_size: NonZeroU32,
+    ) -> usize {
         let mut expanded_chars = 0;
         let mut expanded_bytes = 0;
         let mut collapsed_bytes = 0;
@@ -260,7 +264,8 @@ impl TabSnapshot {
                 break;
             }
             if c == '\t' {
-                let tab_len = tab_size as usize - expanded_chars % tab_size as usize;
+                let tab_size = tab_size.get() as usize;
+                let tab_len = tab_size - expanded_chars % tab_size;
                 expanded_bytes += tab_len;
                 expanded_chars += tab_len;
             } else {
@@ -276,7 +281,7 @@ impl TabSnapshot {
         mut chars: impl Iterator<Item = char>,
         column: usize,
         bias: Bias,
-        tab_size: u32,
+        tab_size: NonZeroU32,
     ) -> (usize, usize, usize) {
         let mut expanded_bytes = 0;
         let mut expanded_chars = 0;
@@ -287,7 +292,8 @@ impl TabSnapshot {
             }
 
             if c == '\t' {
-                let tab_len = tab_size as usize - (expanded_chars % tab_size as usize);
+                let tab_size = tab_size.get() as usize;
+                let tab_len = tab_size - (expanded_chars % tab_size);
                 expanded_chars += tab_len;
                 expanded_bytes += tab_len;
                 if expanded_bytes > column {
@@ -400,7 +406,7 @@ pub struct TabChunks<'a> {
     column: usize,
     output_position: Point,
     max_output_position: Point,
-    tab_size: u32,
+    tab_size: NonZeroU32,
     skip_leading_tab: bool,
 }
 
@@ -432,7 +438,8 @@ impl<'a> Iterator for TabChunks<'a> {
                         });
                     } else {
                         self.chunk.text = &self.chunk.text[1..];
-                        let mut len = self.tab_size - self.column as u32 % self.tab_size;
+                        let tab_size = self.tab_size.get() as u32;
+                        let mut len = tab_size - self.column as u32 % tab_size;
                         let next_output_position = cmp::min(
                             self.output_position + Point::new(0, len),
                             self.max_output_position,
@@ -470,14 +477,23 @@ mod tests {
 
     #[test]
     fn test_expand_tabs() {
-        assert_eq!(TabSnapshot::expand_tabs("\t".chars(), 0, 4), 0);
-        assert_eq!(TabSnapshot::expand_tabs("\t".chars(), 1, 4), 4);
-        assert_eq!(TabSnapshot::expand_tabs("\ta".chars(), 2, 4), 5);
+        assert_eq!(
+            TabSnapshot::expand_tabs("\t".chars(), 0, 4.try_into().unwrap()),
+            0
+        );
+        assert_eq!(
+            TabSnapshot::expand_tabs("\t".chars(), 1, 4.try_into().unwrap()),
+            4
+        );
+        assert_eq!(
+            TabSnapshot::expand_tabs("\ta".chars(), 2, 4.try_into().unwrap()),
+            5
+        );
     }
 
     #[gpui::test(iterations = 100)]
     fn test_random_tabs(cx: &mut gpui::MutableAppContext, mut rng: StdRng) {
-        let tab_size = rng.gen_range(1..=4);
+        let tab_size = NonZeroU32::new(rng.gen_range(1..=4)).unwrap();
         let len = rng.gen_range(0..30);
         let buffer = if rng.gen() {
             let text = RandomCharIter::new(&mut rng).take(len).collect::<String>();
@@ -529,7 +545,7 @@ mod tests {
             );
 
             let mut actual_summary = tabs_snapshot.text_summary_for_range(start..end);
-            if tab_size > 1 && folds_snapshot.text().contains('\t') {
+            if tab_size.get() > 1 && folds_snapshot.text().contains('\t') {
                 actual_summary.longest_row = expected_summary.longest_row;
                 actual_summary.longest_row_chars = expected_summary.longest_row_chars;
             }

crates/editor/src/display_map/wrap_map.rs 🔗

@@ -1033,7 +1033,7 @@ mod tests {
     use rand::prelude::*;
     use settings::Settings;
     use smol::stream::StreamExt;
-    use std::{cmp, env};
+    use std::{cmp, env, num::NonZeroU32};
     use text::Rope;
 
     #[gpui::test(iterations = 100)]
@@ -1052,7 +1052,7 @@ mod tests {
         } else {
             Some(rng.gen_range(0.0..=1000.0))
         };
-        let tab_size = rng.gen_range(1..=4);
+        let tab_size = NonZeroU32::new(rng.gen_range(1..=4)).unwrap();
         let family_id = font_cache.load_family(&["Helvetica"]).unwrap();
         let font_id = font_cache
             .select_font(family_id, &Default::default())
@@ -1194,7 +1194,7 @@ mod tests {
                     log::info!("{} summary: {:?}", ix, item.summary.output,);
                 }
 
-                if tab_size == 1
+                if tab_size.get() == 1
                     || !wrapped_snapshot
                         .tab_snapshot
                         .fold_snapshot

crates/editor/src/editor.rs 🔗

@@ -55,6 +55,7 @@ use std::{
     borrow::Cow,
     cmp::{self, Ordering, Reverse},
     mem,
+    num::NonZeroU32,
     ops::{Deref, DerefMut, Range, RangeInclusive},
     sync::Arc,
     time::{Duration, Instant},
@@ -2793,9 +2794,10 @@ impl Editor {
                             IndentKind::Space => {
                                 cx.global::<Settings>().tab_size(language_name.as_deref())
                             }
-                            IndentKind::Tab => 1,
+                            IndentKind::Tab => NonZeroU32::new(1).unwrap(),
                         };
                         if old_head.column <= indent_size.len && old_head.column > 0 {
+                            let indent_len = indent_len.get();
                             new_head = cmp::min(
                                 new_head,
                                 Point::new(
@@ -2856,7 +2858,7 @@ impl Editor {
                         let tab_size = if settings.hard_tabs(language_name.as_deref()) {
                             IndentSize::tab()
                         } else {
-                            let tab_size = settings.tab_size(language_name.as_deref());
+                            let tab_size = settings.tab_size(language_name.as_deref()).get();
                             let char_column = buffer
                                 .read(cx)
                                 .text_for_range(Point::new(selection.start.row, 0)..selection.start)
@@ -2894,7 +2896,7 @@ impl Editor {
                 for selection in &mut selections {
                     let language_name = buffer.language_at(selection.start, cx).map(|l| l.name());
                     let settings = &cx.global::<Settings>();
-                    let tab_size = settings.tab_size(language_name.as_deref());
+                    let tab_size = settings.tab_size(language_name.as_deref()).get();
                     let indent_kind = if settings.hard_tabs(language_name.as_deref()) {
                         IndentKind::Tab
                     } else {
@@ -2973,7 +2975,10 @@ impl Editor {
             let snapshot = buffer.snapshot(cx);
             for selection in &selections {
                 let language_name = buffer.language_at(selection.start, cx).map(|l| l.name());
-                let tab_size = cx.global::<Settings>().tab_size(language_name.as_deref());
+                let tab_size = cx
+                    .global::<Settings>()
+                    .tab_size(language_name.as_deref())
+                    .get();
                 let mut rows = selection.spanned_rows(false, &display_map);
 
                 // Avoid re-outdenting a row that has already been outdented by a
@@ -7583,14 +7588,14 @@ mod tests {
                 .with_language_defaults(
                     "TOML",
                     LanguageSettings {
-                        tab_size: Some(2),
+                        tab_size: Some(2.try_into().unwrap()),
                         ..Default::default()
                     },
                 )
                 .with_language_defaults(
                     "Rust",
                     LanguageSettings {
-                        tab_size: Some(4),
+                        tab_size: Some(4.try_into().unwrap()),
                         ..Default::default()
                     },
                 ),
@@ -9163,7 +9168,7 @@ mod tests {
                 settings.language_overrides.insert(
                     "Rust".into(),
                     LanguageSettings {
-                        tab_size: Some(8),
+                        tab_size: Some(8.try_into().unwrap()),
                         ..Default::default()
                     },
                 );
@@ -9277,7 +9282,7 @@ mod tests {
                 settings.language_overrides.insert(
                     "Rust".into(),
                     LanguageSettings {
-                        tab_size: Some(8),
+                        tab_size: Some(8.try_into().unwrap()),
                         ..Default::default()
                     },
                 );

crates/editor/src/multi_buffer.rs 🔗

@@ -347,7 +347,7 @@ impl MultiBuffer {
                     let indent_size = if settings.hard_tabs(language_name.as_deref()) {
                         IndentSize::tab()
                     } else {
-                        IndentSize::spaces(settings.tab_size(language_name.as_deref()))
+                        IndentSize::spaces(settings.tab_size(language_name.as_deref()).get())
                     };
                     buffer.edit_with_autoindent(edits, indent_size, cx);
                 } else {
@@ -473,7 +473,7 @@ impl MultiBuffer {
                         let indent_size = if settings.hard_tabs(language_name.as_deref()) {
                             IndentSize::tab()
                         } else {
-                            IndentSize::spaces(settings.tab_size(language_name.as_deref()))
+                            IndentSize::spaces(settings.tab_size(language_name.as_deref()).get())
                         };
 
                         buffer.edit_with_autoindent(deletions, indent_size, cx);

crates/project/src/project.rs 🔗

@@ -2848,7 +2848,7 @@ impl Project {
                         .request::<lsp::request::Formatting>(lsp::DocumentFormattingParams {
                             text_document,
                             options: lsp::FormattingOptions {
-                                tab_size,
+                                tab_size: tab_size.into(),
                                 insert_spaces: true,
                                 insert_final_newline: Some(true),
                                 ..Default::default()
@@ -2870,7 +2870,7 @@ impl Project {
                                 text_document,
                                 range: lsp::Range::new(buffer_start, buffer_end),
                                 options: lsp::FormattingOptions {
-                                    tab_size,
+                                    tab_size: tab_size.into(),
                                     insert_spaces: true,
                                     insert_final_newline: Some(true),
                                     ..Default::default()

crates/settings/src/settings.rs 🔗

@@ -11,7 +11,7 @@ use schemars::{
 };
 use serde::{de::DeserializeOwned, Deserialize};
 use serde_json::Value;
-use std::{collections::HashMap, sync::Arc};
+use std::{collections::HashMap, num::NonZeroU32, sync::Arc};
 use theme::{Theme, ThemeRegistry};
 use util::ResultExt as _;
 
@@ -32,7 +32,7 @@ pub struct Settings {
 
 #[derive(Clone, Debug, Default, Deserialize, JsonSchema)]
 pub struct LanguageSettings {
-    pub tab_size: Option<u32>,
+    pub tab_size: Option<NonZeroU32>,
     pub hard_tabs: Option<bool>,
     pub soft_wrap: Option<SoftWrap>,
     pub preferred_line_length: Option<u32>,
@@ -99,9 +99,9 @@ impl Settings {
         self
     }
 
-    pub fn tab_size(&self, language: Option<&str>) -> u32 {
+    pub fn tab_size(&self, language: Option<&str>) -> NonZeroU32 {
         self.language_setting(language, |settings| settings.tab_size)
-            .unwrap_or(4)
+            .unwrap_or(4.try_into().unwrap())
     }
 
     pub fn hard_tabs(&self, language: Option<&str>) -> bool {

crates/zed/src/main.rs 🔗

@@ -83,21 +83,21 @@ fn main() {
         .with_language_defaults(
             "C",
             settings::LanguageSettings {
-                tab_size: Some(2),
+                tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         )
         .with_language_defaults(
             "C++",
             settings::LanguageSettings {
-                tab_size: Some(2),
+                tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         )
         .with_language_defaults(
             "Go",
             settings::LanguageSettings {
-                tab_size: Some(4),
+                tab_size: Some(4.try_into().unwrap()),
                 hard_tabs: Some(true),
                 ..Default::default()
             },
@@ -112,28 +112,28 @@ fn main() {
         .with_language_defaults(
             "Rust",
             settings::LanguageSettings {
-                tab_size: Some(4),
+                tab_size: Some(4.try_into().unwrap()),
                 ..Default::default()
             },
         )
         .with_language_defaults(
             "JavaScript",
             settings::LanguageSettings {
-                tab_size: Some(2),
+                tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         )
         .with_language_defaults(
             "TypeScript",
             settings::LanguageSettings {
-                tab_size: Some(2),
+                tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         )
         .with_language_defaults(
             "TSX",
             settings::LanguageSettings {
-                tab_size: Some(2),
+                tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         );

crates/zed/src/settings_file.rs 🔗

@@ -128,7 +128,7 @@ mod tests {
         let settings = cx.read(Settings::test).with_language_defaults(
             "JavaScript",
             LanguageSettings {
-                tab_size: Some(2),
+                tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         );
@@ -156,9 +156,9 @@ mod tests {
         assert_eq!(settings.preferred_line_length(Some("Markdown")), 100);
         assert_eq!(settings.preferred_line_length(Some("JavaScript")), 80);
 
-        assert_eq!(settings.tab_size(None), 8);
-        assert_eq!(settings.tab_size(Some("Markdown")), 2);
-        assert_eq!(settings.tab_size(Some("JavaScript")), 8);
+        assert_eq!(settings.tab_size(None).get(), 8);
+        assert_eq!(settings.tab_size(Some("Markdown")).get(), 2);
+        assert_eq!(settings.tab_size(Some("JavaScript")).get(), 8);
 
         fs.save(
             "/settings2.json".as_ref(),
@@ -192,9 +192,9 @@ mod tests {
         assert_eq!(settings.preferred_line_length(Some("Markdown")), 120);
         assert_eq!(settings.preferred_line_length(Some("JavaScript")), 80);
 
-        assert_eq!(settings.tab_size(None), 2);
-        assert_eq!(settings.tab_size(Some("Markdown")), 2);
-        assert_eq!(settings.tab_size(Some("JavaScript")), 2);
+        assert_eq!(settings.tab_size(None).get(), 2);
+        assert_eq!(settings.tab_size(Some("Markdown")).get(), 2);
+        assert_eq!(settings.tab_size(Some("JavaScript")).get(), 2);
 
         fs.remove_file("/settings2.json".as_ref(), Default::default())
             .await
@@ -217,8 +217,8 @@ mod tests {
         assert_eq!(settings.preferred_line_length(Some("Markdown")), 100);
         assert_eq!(settings.preferred_line_length(Some("JavaScript")), 80);
 
-        assert_eq!(settings.tab_size(None), 8);
-        assert_eq!(settings.tab_size(Some("Markdown")), 2);
-        assert_eq!(settings.tab_size(Some("JavaScript")), 8);
+        assert_eq!(settings.tab_size(None).get(), 8);
+        assert_eq!(settings.tab_size(Some("Markdown")).get(), 2);
+        assert_eq!(settings.tab_size(Some("JavaScript")).get(), 8);
     }
 }

crates/zed/src/zed.rs 🔗

@@ -166,6 +166,9 @@ pub fn initialize_workspace(
         let action_names = cx.all_action_names().collect::<Vec<_>>();
         project.set_language_server_settings(serde_json::json!({
             "json": {
+                "format": {
+                    "enable": true,
+                },
                 "schemas": [
                     {
                         "fileMatch": [".zed/settings.json"],