Skip to content

Commit

Permalink
Appease most of the Clippy lints (#673)
Browse files Browse the repository at this point in the history
Part of #155 

These should all be more or less obvious, maybe with the exception of:
- ~`comparison_chain` - in this case it might be more noise than it's
worth, I'm fine with silencing it here, instead~
  EDIT: Actually, let's drop it for now
- `single_char_pattern` - in our case we do less `"`-escaping, so that's
good enough for me (but I know it might be considered controversial)
- `clippy::upper_case_acronyms` - this helps follow the Rust naming
convention (`CLI` -> `Cli`, which I'm strongly in favour; it's not a
SCREAMING_CASE_CONSTANT)

Some select lints are explicitly allowed on a case-by-case basis here
but in general are worthwhile to be linted against, e.g.
`enum_variant_names` or `manual_range_contains`.

The only ones left now are:
- `comparison_chain` - in this case it might be more noise than it's
worth, I'm fine with silencing it here, instead
- `needless_return`, which we will have *many* instances of and I'd like
to have a separate PR for that (but I'm *strongly* in favour, as I find
it to be one of the signature aspects of Rust)
- `should_implement_trait` - needs to be silenced in one case for
`#[napi] fn clone`, but `ParserContext::next` might or might not
implement the `Iterator`, I'm not sure yet, so I'd like to defer it for
now.
  • Loading branch information
Xanewok committed Nov 28, 2023
1 parent 0434b68 commit 6447e03
Show file tree
Hide file tree
Showing 85 changed files with 328 additions and 405 deletions.
65 changes: 0 additions & 65 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,74 +12,9 @@ rustflags = [
# This is a list of allowed Clippy rules for the purposes of gradual migration.
# See https://github.com/NomicFoundation/slang/pull/626
"-A",
"clippy::bool_assert_comparison",
"-A",
"clippy::cmp_owned",
"-A",
"clippy::collapsible_if",
"-A",
"clippy::comparison_chain",
"-A",
"clippy::enum_variant_names",
"-A",
"clippy::expect_fun_call",
"-A",
"clippy::explicit_auto_deref",
"-A",
"clippy::from_over_into",
"-A",
"clippy::inherent_to_string",
"-A",
"clippy::into_iter_on_ref",
"-A",
"clippy::len_without_is_empty",
"-A",
"clippy::len_zero",
"-A",
"clippy::manual_range_contains",
"-A",
"clippy::match_like_matches_macro",
"-A",
"clippy::needless_borrow",
"-A",
"clippy::needless_range_loop",
"-A",
"clippy::needless_return",
"-A",
"clippy::new_without_default",
"-A",
"clippy::println_empty_string",
"-A",
"clippy::ptr_arg",
"-A",
"clippy::redundant_closure",
"-A",
"clippy::redundant_pattern",
"-A",
"clippy::redundant_pattern_matching",
"-A",
"clippy::redundant_static_lifetimes",
"-A",
"clippy::should_implement_trait",
"-A",
"clippy::single_char_add_str",
"-A",
"clippy::single_char_pattern",
"-A",
"clippy::to_string_in_format_args",
"-A",
"clippy::upper_case_acronyms",
"-A",
"clippy::useless_asref",
"-A",
"clippy::useless_conversion",
"-A",
"clippy::useless_format",
"-A",
"clippy::write_literal",
"-A",
"clippy::writeln_empty_string",
"-A",
"clippy::wrong_self_convention",

]
16 changes: 8 additions & 8 deletions crates/codegen/ebnf/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,29 @@ impl EbnfNode {
} => {
return Self::sequence(vec![
Self::production_ref(&open.reference),
Self::from_parser(&parser),
Self::from_parser(parser),
Self::production_ref(&close.reference),
]);
}

ParserDefinition::OneOrMore(parser) => {
return Self::one_or_more(Self::from_parser(&parser));
return Self::one_or_more(Self::from_parser(parser));
}

ParserDefinition::Optional(parser) => {
return Self::optional(Self::from_parser(&parser));
return Self::optional(Self::from_parser(parser));
}

ParserDefinition::Reference(name) => {
return Self::production_ref(&name);
return Self::production_ref(name);
}

ParserDefinition::SeparatedBy { parser, separator } => {
return Self::sequence(vec![
Self::from_parser(&parser),
Self::from_parser(parser),
Self::zero_or_more(Self::sequence(vec![
Self::production_ref(&separator.reference),
Self::from_parser(&parser),
Self::from_parser(parser),
])),
]);
}
Expand All @@ -49,13 +49,13 @@ impl EbnfNode {

ParserDefinition::TerminatedBy { parser, terminator } => {
return Self::sequence(vec![
Self::from_parser(&parser),
Self::from_parser(parser),
Self::production_ref(&terminator.reference),
]);
}

ParserDefinition::ZeroOrMore(parser) => {
return Self::zero_or_more(Self::from_parser(&parser));
return Self::zero_or_more(Self::from_parser(parser));
}
};
}
Expand Down
30 changes: 15 additions & 15 deletions crates/codegen/ebnf/src/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ impl EbnfSerializer {
buffer.push_str(&self.display_name(name));
buffer.push_str(" = ");
buffer.push_str(&self.serialize_root_node(name, root_node));
buffer.push_str(";");
buffer.push(';');

match self.outputs.get_mut(name) {
Some(existing) => {
if !existing.is_empty() {
existing.push_str("\n");
existing.push('\n');
}

existing.push_str(&buffer);
Expand Down Expand Up @@ -127,7 +127,7 @@ impl EbnfSerializer {
pub fn serialize_node(&mut self, top_node: &EbnfNode, buffer: &mut String) {
match top_node {
EbnfNode::Choice { nodes } => {
for (i, node) in nodes.into_iter().enumerate() {
for (i, node) in nodes.iter().enumerate() {
if i > 0 {
buffer.push_str(" | ");
}
Expand All @@ -144,29 +144,29 @@ impl EbnfSerializer {
self.serialize_child_node(top_node, subtrahend, buffer);
}
EbnfNode::Not { node } => {
buffer.push_str("!");
buffer.push('!');
self.serialize_child_node(top_node, node, buffer);
}
EbnfNode::OneOrMore { node } => {
self.serialize_child_node(top_node, node, buffer);
buffer.push_str("+");
buffer.push('+');
}
EbnfNode::Optional { node } => {
self.serialize_child_node(top_node, node, buffer);
buffer.push_str("?");
buffer.push('?');
}
EbnfNode::Range { from, to } => {
buffer.push_str(&format_string_literal(&from.to_string()));
buffer.push_str("…");
buffer.push('…');
buffer.push_str(&format_string_literal(&to.to_string()));
}
EbnfNode::ProductionRef { name } => {
buffer.push_str(&self.display_name(name));
}
EbnfNode::Sequence { nodes } => {
for (i, node) in nodes.into_iter().enumerate() {
for (i, node) in nodes.iter().enumerate() {
if i > 0 {
buffer.push_str(" ");
buffer.push(' ');
}

self.serialize_child_node(top_node, node, buffer);
Expand All @@ -178,22 +178,22 @@ impl EbnfSerializer {
EbnfNode::WithComment { node, comment } => {
self.serialize_child_node(top_node, node, buffer);
buffer.push_str(" (* ");
buffer.push_str(&comment);
buffer.push_str(comment);
buffer.push_str(" *)");
}
EbnfNode::ZeroOrMore { node } => {
self.serialize_child_node(top_node, node, buffer);
buffer.push_str("*");
buffer.push('*');
}
};
}

fn serialize_child_node(&mut self, parent: &EbnfNode, child: &EbnfNode, buffer: &mut String) {
if discriminant(parent) != discriminant(child) && child.precedence() <= parent.precedence()
{
buffer.push_str("(");
buffer.push('(');
self.serialize_node(child, buffer);
buffer.push_str(")");
buffer.push(')');
} else {
self.serialize_node(child, buffer);
}
Expand Down Expand Up @@ -224,7 +224,7 @@ impl EbnfSerializer {
}

fn format_string_literal(value: &str) -> String {
let delimiter = if value.contains('"') && !value.contains("'") {
let delimiter = if value.contains('"') && !value.contains('\'') {
'\''
} else {
'"'
Expand All @@ -241,7 +241,7 @@ fn format_string_literal(value: &str) -> String {
_ => {
panic!(
"Unexpected character in string literal: '{c}'",
c = c.escape_unicode().to_string()
c = c.escape_unicode()
);
}
})
Expand Down
24 changes: 12 additions & 12 deletions crates/codegen/grammar/src/grammar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,27 +50,27 @@ impl GrammarElement {
}
}

impl Into<GrammarElement> for ScannerDefinitionRef {
fn into(self) -> GrammarElement {
GrammarElement::ScannerDefinition(self)
impl From<ScannerDefinitionRef> for GrammarElement {
fn from(def: ScannerDefinitionRef) -> Self {
GrammarElement::ScannerDefinition(def)
}
}

impl Into<GrammarElement> for TriviaParserDefinitionRef {
fn into(self) -> GrammarElement {
GrammarElement::TriviaParserDefinition(self)
impl From<TriviaParserDefinitionRef> for GrammarElement {
fn from(def: TriviaParserDefinitionRef) -> Self {
GrammarElement::TriviaParserDefinition(def)
}
}

impl Into<GrammarElement> for ParserDefinitionRef {
fn into(self) -> GrammarElement {
GrammarElement::ParserDefinition(self)
impl From<ParserDefinitionRef> for GrammarElement {
fn from(def: ParserDefinitionRef) -> Self {
GrammarElement::ParserDefinition(def)
}
}

impl Into<GrammarElement> for PrecedenceParserDefinitionRef {
fn into(self) -> GrammarElement {
GrammarElement::PrecedenceParserDefinition(self)
impl From<PrecedenceParserDefinitionRef> for GrammarElement {
fn from(def: PrecedenceParserDefinitionRef) -> Self {
GrammarElement::PrecedenceParserDefinition(def)
}
}

Expand Down
24 changes: 12 additions & 12 deletions crates/codegen/grammar/src/parser_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,27 @@ pub enum ParserDefinitionNode {
TerminatedBy(Box<Self>, Box<Self>),
}

impl Into<ParserDefinitionNode> for ScannerDefinitionRef {
fn into(self) -> ParserDefinitionNode {
ParserDefinitionNode::ScannerDefinition(self)
impl From<ScannerDefinitionRef> for ParserDefinitionNode {
fn from(def: ScannerDefinitionRef) -> Self {
ParserDefinitionNode::ScannerDefinition(def)
}
}

impl Into<ParserDefinitionNode> for TriviaParserDefinitionRef {
fn into(self) -> ParserDefinitionNode {
ParserDefinitionNode::TriviaParserDefinition(self)
impl From<TriviaParserDefinitionRef> for ParserDefinitionNode {
fn from(def: TriviaParserDefinitionRef) -> Self {
ParserDefinitionNode::TriviaParserDefinition(def)
}
}

impl Into<ParserDefinitionNode> for ParserDefinitionRef {
fn into(self) -> ParserDefinitionNode {
ParserDefinitionNode::ParserDefinition(self)
impl From<ParserDefinitionRef> for ParserDefinitionNode {
fn from(def: ParserDefinitionRef) -> Self {
ParserDefinitionNode::ParserDefinition(def)
}
}

impl Into<ParserDefinitionNode> for PrecedenceParserDefinitionRef {
fn into(self) -> ParserDefinitionNode {
ParserDefinitionNode::PrecedenceParserDefinition(self)
impl From<PrecedenceParserDefinitionRef> for ParserDefinitionNode {
fn from(def: PrecedenceParserDefinitionRef) -> Self {
ParserDefinitionNode::PrecedenceParserDefinition(def)
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/codegen/grammar/src/scanner_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ pub enum ScannerDefinitionNode {
ScannerDefinition(ScannerDefinitionRef),
}

impl Into<ScannerDefinitionNode> for ScannerDefinitionRef {
fn into(self) -> ScannerDefinitionNode {
ScannerDefinitionNode::ScannerDefinition(self)
impl From<ScannerDefinitionRef> for ScannerDefinitionNode {
fn from(def_ref: ScannerDefinitionRef) -> Self {
ScannerDefinitionNode::ScannerDefinition(def_ref)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ fn calculate_defined_in(analysis: &mut Analysis, item: &Item) -> VersionSet {
return defined_in;
}

#[allow(clippy::enum_variant_names)]
#[derive(thiserror::Error, Debug)]
enum Errors<'err> {
#[error("An item with the name '{0}' already exists.")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn check_unreachabable_items(analysis: &mut Analysis) {
if !metadata.defined_in.is_empty() && !visited.contains(&*metadata.name) {
analysis
.errors
.add(&metadata.name, &Errors::Unreachable(&*metadata.name));
.add(&metadata.name, &Errors::Unreachable(&metadata.name));
}
}
}
Expand Down

0 comments on commit 6447e03

Please sign in to comment.