Skip to content

Commit

Permalink
fix: inline-comment detection logic (#219)
Browse files Browse the repository at this point in the history
  • Loading branch information
a-frantz authored Oct 16, 2024
1 parent 0e0f278 commit da3342c
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 47 deletions.
5 changes: 5 additions & 0 deletions wdl-lint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Some `warning` diagnostics are now `note` diagnostics ([#187](https://github.com/stjude-rust-labs/wdl/pull/187))

### Added

* Added comments to the trailing whitespace check of the `Whitespace` rule ([#177](https://github.com/stjude-rust-labs/wdl/pull/177))
* Added a `MalformedLintDirective` rule ([#194](https://github.com/stjude-rust-labs/wdl/pull/194))

### Fixed

* Fixed inline comment detection edge case ([#219](https://github.com/stjude-rust-labs/wdl/pull/219))

## 0.6.0 - 09-16-2024

### Fixed
Expand Down
33 changes: 0 additions & 33 deletions wdl-lint/src/rules/comment_whitespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,39 +266,6 @@ mod tests {
use wdl_ast::SyntaxKind;
use wdl_ast::SyntaxTree;

use crate::rules::comment_whitespace::is_inline_comment;

#[test]
fn it_detects_inline() {
let (tree, _) = SyntaxTree::parse(
r#"version 1.2
task foo { # an in-line comment
# not an in-line comment
}"#,
);

let mut comments = tree
.root()
.descendants_with_tokens()
.filter(|t| t.kind() == SyntaxKind::Comment);

let inline_comment = comments.next().expect("there should be a first comment");
let inline_comment = Comment::cast(inline_comment.as_token().unwrap().clone()).unwrap();

let is_inline = is_inline_comment(&inline_comment);

assert!(is_inline);

let non_inline_comment = comments.next().expect("there should be a second comment");
let non_inline_comment =
Comment::cast(non_inline_comment.as_token().unwrap().clone()).unwrap();

let is_inline = is_inline_comment(&non_inline_comment);

assert!(!is_inline);
}

#[test]
fn filter_parents() {
let (tree, _) = SyntaxTree::parse(
Expand Down
57 changes: 51 additions & 6 deletions wdl-lint/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,18 @@ use wdl_ast::SyntaxKind;
/// whitespace.
pub fn is_inline_comment(token: &Comment) -> bool {
if let Some(prior) = token.syntax().prev_sibling_or_token() {
return prior.kind() != SyntaxKind::Whitespace
|| !prior
.as_token()
.expect("should be a token")
.text()
.contains('\n');
let whitespace = prior.kind() == SyntaxKind::Whitespace;
if !whitespace {
return true;
}

let contains_newline = prior
.as_token()
.expect("whitespace should be a token")
.text()
.contains('\n');
let first = prior.prev_sibling_or_token().is_none();
return !contains_newline && !first;
}
false
}
Expand Down Expand Up @@ -65,6 +71,45 @@ mod test {

use super::*;

#[test]
fn it_detects_inline() {
let (tree, _) = wdl_ast::SyntaxTree::parse(
r#" # not an in-line comment
version 1.2
task foo { # an in-line comment
# not an in-line comment
}"#,
);

let mut comments = tree
.root()
.descendants_with_tokens()
.filter(|t| t.kind() == SyntaxKind::Comment);

let first = comments.next().expect("there should be a first comment");
let first = Comment::cast(first.as_token().unwrap().clone()).unwrap();

let is_inline = is_inline_comment(&first);

assert!(!is_inline);

let second = comments.next().expect("there should be a second comment");
let second = Comment::cast(second.as_token().unwrap().clone()).unwrap();

let is_inline = is_inline_comment(&second);

assert!(is_inline);

let third = comments.next().expect("there should be a third comment");
let third = Comment::cast(third.as_token().unwrap().clone()).unwrap();

let is_inline = is_inline_comment(&third);

assert!(!is_inline);
}

#[test]
fn test_strip_newline() {
let s = "this has no newline";
Expand Down
8 changes: 0 additions & 8 deletions wdl-lint/tests/lints/preamble-ws/source.errors
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@ note[PreambleFormatting]: unnecessary whitespace in document preamble
1 │ #@ except: DescriptionMissing
│ ^^^^

warning[MalformedLintDirective]: lint directive must be on its own line
┌─ tests/lints/preamble-ws/source.wdl:1:5
1 │ #@ except: DescriptionMissing
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= fix: move the lint directive to its own line

note[PreambleFormatting]: expected exactly one blank line between lint directives and preamble comments
┌─ tests/lints/preamble-ws/source.wdl:1:34
Expand Down

0 comments on commit da3342c

Please sign in to comment.