Fork of daniellemaywood.uk/gleam — Wasm codegen work
2

Configure Feed

Select the types of activity you want to include in your feed.

Fix bug with import sorting

+100 -73
+1 -1
CHANGELOG.md
··· 72 72 needlessly indented if preceded by a comment. 73 73 - Line endings other than `\n` are now handled by the formatter, preserving 74 74 blank lines and converting them to `\n`. 75 + - The formatter can now format groups of imports alphabetically. 75 76 76 77 ### Build tool 77 78 ··· 151 152 152 153 - The format used by the formatter has been improved in some niche cases. 153 154 - Improved the formatting of long case guards. 154 - - The formatter can now format groups of imports alphabetically. 155 155 156 156 ## v1.0.0-rc1 - 2024-02-10 157 157
+72 -72
compiler-core/src/format.rs
··· 219 219 /// Separates the imports in groups delimited by comments or empty lines and 220 220 /// sorts each group alphabetically. 221 221 /// 222 + /// The formatter needs to play nicely with import groups defined by the 223 + /// programmer. If one puts a comment before an import then that's a clue 224 + /// for the formatter that it has run into a gorup of related imports. 225 + /// 226 + /// So we can't just sort `imports` and format each one, we have to be a 227 + /// bit smarter and see if each import is preceded by a comment. 228 + /// Once we find a comment we know we're done with the current import 229 + /// group and a new one has started. 230 + /// 231 + /// ```gleam 232 + /// // This is an import group. 233 + /// import gleam/int 234 + /// import gleam/string 235 + /// 236 + /// // This marks the beginning of a new import group that can't 237 + /// // be mushed together with the previous one! 238 + /// import wibble 239 + /// import wobble 240 + /// ``` 222 241 fn imports<'a>(&mut self, imports: Vec<&'a TargetedDefinition>) -> Vec<Document<'a>> { 223 - // The formatter needs to play nicely with import groups defined by the 224 - // programmer. If one puts a comment before an import then that's a clue 225 - // for the formatter that it has run into a gorup of related imports. 226 - // 227 - // So we can't just sort `imports` and format each one, we have to be a 228 - // bit smarter and see if each import is preceded by a comment. 229 - // Once we find a comment we know we're done with the current import 230 - // group and a new one has started. 231 - // 232 - // ```gleam 233 - // // This is an import group. 234 - // import gleam/int 235 - // import gleam/string 236 - // 237 - // // This marks the beginning of a new import group that can't 238 - // // be mushed together with the previous one! 239 - // import wibble 240 - // import wobble 241 - // ``` 242 - let mut documents = vec![]; 242 + let mut import_groups_docs = vec![]; 243 243 let mut current_group = vec![]; 244 - let mut group_comments = None; 244 + let mut current_group_delimiter = docvec!(); 245 245 246 246 for import in imports { 247 247 let start = import.definition.location().start; 248 - // If the import is preceded by a comment then we want to put it 249 - // into a new group and we can print the current one. 250 - if !current_group.is_empty() 251 - && (self.any_comments(start) || self.any_empty_lines(start)) 252 - { 253 - documents.append(&mut self.sorted_import_group(&mut current_group, group_comments)); 254 - documents.push(lines(2)); 255 - // We pop the comment introducing the group and save it for 256 - // later for when the group is over and we can actually print 257 - // it. 258 - // We have to immediately pop the comment as soon as we start 259 - // with a new group or it would still be present in 260 - // `self.comments` messing up `self.any_comments` for the next 261 - // imports in the group. 262 - group_comments = printed_comments(self.pop_comments(start), false); 248 + 249 + // We need to start a new group if the `import` is preceded by one or 250 + // more empty lines or a `//` comment. 251 + let start_new_group = self.any_comments(start) || self.any_empty_lines(start); 252 + if start_new_group { 253 + // First we print the previous group and clear it out to start a 254 + // new empty group containing the import we've just ran into. 255 + if !current_group.is_empty() { 256 + import_groups_docs.push(docvec![ 257 + current_group_delimiter, 258 + self.sorted_import_group(&current_group) 259 + ]); 260 + current_group.clear(); 261 + } 262 + 263 + // Now that we've taken care of the previous group we can start 264 + // the new one. We know it's preceded either by an empty line or 265 + // some comments se we have to be a bit more precise and save the 266 + // actual delimiter that we're going to put at the top of this 267 + // group. 268 + 269 + let comments = self.pop_comments(start); 270 + let _ = self.pop_empty_lines(start); 271 + current_group_delimiter = printed_comments(comments, true).unwrap_or(docvec!()); 263 272 } 264 - let _ = self.pop_empty_lines(start); 273 + // Lastly we add the import to the group. 265 274 current_group.push(import); 266 275 } 267 276 268 277 // Let's not forget about the last import group! 269 278 if !current_group.is_empty() { 270 - documents.append(&mut self.sorted_import_group(&mut current_group, group_comments)); 279 + import_groups_docs.push(docvec![ 280 + current_group_delimiter, 281 + self.sorted_import_group(&current_group) 282 + ]); 271 283 } 272 284 273 - documents 285 + // We want all consecutive import groups to be separated by an empty line. 286 + // This should really be `.intersperse(line())` but I can't do that 287 + // because of https://github.com/rust-lang/rust/issues/48919. 288 + Itertools::intersperse(import_groups_docs.into_iter(), lines(2)).collect_vec() 274 289 } 275 290 276 - /// Prints the imports as a single sorted group of import statements 277 - /// draining the given vector. 278 - /// 279 - /// `group_comment` is the comment preceding the current group. 280 - /// It might be missing since a group could also be defined by simply having 281 - /// an empty line between imports. 291 + /// Prints the imports as a single sorted group of import statements. 282 292 /// 283 - fn sorted_import_group<'a>( 284 - &mut self, 285 - imports: &mut Vec<&'a TargetedDefinition>, 286 - group_comment: Option<Document<'a>>, 287 - ) -> Vec<Document<'a>> { 288 - let mut documents = Vec::with_capacity(imports.len() * 2); 289 - 290 - // If the group is defined with a single comment we print it as the 291 - // first thing. 292 - if let Some(comment) = group_comment { 293 - documents.push(comment) 294 - }; 295 - 296 - imports.sort_by(|one, other| match (&one.definition, &other.definition) { 297 - (Definition::Import(one), Definition::Import(other)) => one.module.cmp(&other.module), 298 - // It shouldn't really be possible for a non import to be here so 299 - // we just return a default value. 300 - _ => std::cmp::Ordering::Equal, 301 - }); 293 + fn sorted_import_group<'a>(&mut self, imports: &Vec<&'a TargetedDefinition>) -> Document<'a> { 294 + let imports = imports 295 + .iter() 296 + .sorted_by(|one, other| match (&one.definition, &other.definition) { 297 + (Definition::Import(one), Definition::Import(other)) => { 298 + one.module.cmp(&other.module) 299 + } 300 + // It shouldn't really be possible for a non import to be here so 301 + // we just return a default value. 302 + _ => std::cmp::Ordering::Equal, 303 + }) 304 + .map(|import| self.targeted_definition(import)); 302 305 303 - for import in imports.iter() { 304 - if !documents.is_empty() { 305 - documents.push(lines(1)) 306 - } 307 - documents.push(self.targeted_definition(import)); 308 - } 309 - imports.clear(); 310 - documents 306 + // This should really be `.intersperse(line())` but I can't do that 307 + // because of https://github.com/rust-lang/rust/issues/48919. 308 + Itertools::intersperse(imports, line()) 309 + .collect_vec() 310 + .to_doc() 311 311 } 312 312 313 313 fn definition<'a>(&mut self, statement: &'a UntypedDefinition) -> Document<'a> {
+27
compiler-core/src/format/tests/imports.rs
··· 185 185 "# 186 186 ); 187 187 } 188 + 189 + // https://github.com/gleam-lang/gleam/issues/2915 190 + #[test] 191 + fn white_lines_between_comments_in_import_groups_are_preserved() { 192 + assert_format!( 193 + r#"import a 194 + 195 + // comment 196 + 197 + import b 198 + "# 199 + ); 200 + } 201 + 202 + // https://github.com/gleam-lang/gleam/issues/2915 203 + #[test] 204 + fn import_sorting_doesnt_add_spurious_white_line() { 205 + assert_format!( 206 + r#"// comment 207 + 208 + import filepath 209 + import gleam/dynamic.{type Dynamic} 210 + import gleam/io 211 + import gleam/list 212 + "# 213 + ); 214 + }