Skip to content

Commit 8f2abb9

Browse files
giacomocavalierilpil
authored andcommitted
emit a single warning for chains of double negations
1 parent 4aec44f commit 8f2abb9

7 files changed

+193
-33
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@
6464
errors.
6565
([Giacomo Cavalieri](https://github.com/giacomocavalieri))
6666

67+
- The compiler now emits a single warning for multiple negations in a row, while
68+
previously it would emit multiple comments highlighting increasingly longer
69+
spans.
70+
([Giacomo Cavalieri](https://github.com/giacomocavalieri))
71+
6772
### Build tool
6873

6974
- New projects are generated using OTP28 on GitHub Actions.

compiler-core/src/type_/expression.rs

Lines changed: 114 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -877,25 +877,65 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
877877
}
878878

879879
fn infer_negate_bool(&mut self, location: SrcSpan, value: UntypedExpr) -> TypedExpr {
880-
let value = self.infer(value);
880+
self.infer_multiple_negate_bool(location, 1, location, value)
881+
}
881882

883+
fn infer_multiple_negate_bool(
884+
&mut self,
885+
starting_location: SrcSpan,
886+
negations: usize,
887+
location: SrcSpan,
888+
value: UntypedExpr,
889+
) -> TypedExpr {
890+
// If we're typing a double negation we just keep going increasing the
891+
// number of consecutive negations, inferring the wrapped value.
892+
if let UntypedExpr::NegateBool {
893+
location: inner_location,
894+
value,
895+
} = value
896+
{
897+
return TypedExpr::NegateBool {
898+
location,
899+
value: Box::new(self.infer_multiple_negate_bool(
900+
starting_location,
901+
negations + 1,
902+
inner_location,
903+
*value,
904+
)),
905+
};
906+
}
907+
908+
// We know the last value can't be a bool negation if we're here, so
909+
// we're ready to produce a typed value!
910+
let value = self.infer(value);
882911
if let Err(error) = unify(bool(), value.type_()) {
883912
self.problems
884-
.error(convert_unify_error(error, value.location()))
913+
.error(convert_unify_error(error, value.location()));
885914
}
886915

887-
if let TypedExpr::NegateBool {
888-
location: inner_location,
889-
..
890-
} = value
891-
{
916+
// If there's more than a single negation we can raise a warning
917+
// highlighting the unneded ones. How many negations are highlighted
918+
// depends if they're an even or odd number:
919+
//
920+
// ```gleam
921+
// !!True // all negations are superfluous.
922+
// !!!True // we can remove all but one negation.
923+
// ```
924+
if negations > 1 {
925+
let location = if negations % 2 == 0 {
926+
SrcSpan {
927+
start: starting_location.start,
928+
end: location.start + 1,
929+
}
930+
} else {
931+
SrcSpan {
932+
start: starting_location.start,
933+
end: location.start,
934+
}
935+
};
936+
892937
self.problems
893-
.warning(Warning::UnnecessaryDoubleBoolNegation {
894-
location: SrcSpan {
895-
start: location.start,
896-
end: inner_location.start + 1,
897-
},
898-
});
938+
.warning(Warning::UnnecessaryDoubleBoolNegation { location });
899939
}
900940

901941
TypedExpr::NegateBool {
@@ -905,41 +945,82 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
905945
}
906946

907947
fn infer_negate_int(&mut self, location: SrcSpan, value: UntypedExpr) -> TypedExpr {
908-
let value = self.infer(value);
948+
self.infer_multiple_negate_int(location, 1, location, value)
949+
}
909950

951+
fn infer_multiple_negate_int(
952+
&mut self,
953+
starting_location: SrcSpan,
954+
mut negations: usize,
955+
location: SrcSpan,
956+
value: UntypedExpr,
957+
) -> TypedExpr {
958+
// If we're typing a double negation we just keep going increasing the
959+
// number of consecutive negations, inferring the wrapped value.
960+
if let UntypedExpr::NegateInt {
961+
location: inner_location,
962+
value,
963+
} = value
964+
{
965+
return TypedExpr::NegateInt {
966+
location,
967+
value: Box::new(self.infer_multiple_negate_int(
968+
starting_location,
969+
negations + 1,
970+
inner_location,
971+
*value,
972+
)),
973+
};
974+
}
975+
976+
// We know the last value can't be an int negation, so we're ready to
977+
// produce a typed value!
978+
let value = self.infer(value);
910979
if let Err(error) = unify(int(), value.type_()) {
911980
self.problems
912981
.error(convert_unify_error(error, value.location()));
913982
}
914983

984+
// This is used to emit a warning in case there's multiple negations.
985+
let mut end = location.start;
986+
987+
// There's one special case where the final integer being typed might be
988+
// negated as well, in that case we need to update the number of
989+
// consecutive negations.
915990
if let TypedExpr::Int {
916991
value: ref v,
917-
location: ref inner_location,
992+
ref location,
918993
..
919994
} = value
920995
&& v.starts_with('-')
921996
{
922-
self.problems
923-
.warning(Warning::UnnecessaryDoubleIntNegation {
924-
location: SrcSpan {
925-
start: location.start,
926-
end: inner_location.start + 1,
927-
},
928-
});
997+
negations += 1;
998+
end = location.start;
929999
}
9301000

931-
if let TypedExpr::NegateInt {
932-
location: inner_location,
933-
..
934-
} = value
935-
{
1001+
// If there's more than a single negation we can raise a warning
1002+
// highlighting the unneded ones. How many negations are highlighted
1003+
// depends if they're an even or odd number:
1004+
//
1005+
// ```gleam
1006+
// --1 // all negations are superfluous.
1007+
// ---1 // we can remove all but one negation.
1008+
// ```
1009+
if negations > 1 {
1010+
let location = if negations % 2 == 0 {
1011+
SrcSpan {
1012+
start: starting_location.start,
1013+
end: end + 1,
1014+
}
1015+
} else {
1016+
SrcSpan {
1017+
start: starting_location.start,
1018+
end,
1019+
}
1020+
};
1021+
9361022
self.problems
937-
.warning(Warning::UnnecessaryDoubleIntNegation {
938-
location: SrcSpan {
939-
start: location.start,
940-
end: inner_location.start + 1,
941-
},
942-
});
1023+
.warning(Warning::UnnecessaryDoubleIntNegation { location });
9431024
}
9441025

9451026
TypedExpr::NegateInt {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "pub fn main() { let _ = !!!!True }"
4+
---
5+
----- SOURCE CODE
6+
pub fn main() { let _ = !!!!True }
7+
8+
----- WARNING
9+
warning: Unnecessary double negation (!!) on bool
10+
┌─ /src/warning/wrn.gleam:1:25
11+
12+
1pub fn main() { let _ = !!!!True }
13+
^^^^ You can safely remove this.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "pub fn main() { let _ = ----7 }"
4+
---
5+
----- SOURCE CODE
6+
pub fn main() { let _ = ----7 }
7+
8+
----- WARNING
9+
warning: Unnecessary double negation (--) on integer
10+
┌─ /src/warning/wrn.gleam:1:25
11+
12+
1pub fn main() { let _ = ----7 }
13+
^^^^ You can safely remove this.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "pub fn main() { let _ = !!!!!False }"
4+
---
5+
----- SOURCE CODE
6+
pub fn main() { let _ = !!!!!False }
7+
8+
----- WARNING
9+
warning: Unnecessary double negation (!!) on bool
10+
┌─ /src/warning/wrn.gleam:1:25
11+
12+
1pub fn main() { let _ = !!!!!False }
13+
^^^^ You can safely remove this.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "pub fn main() { let _ = -----7 }"
4+
---
5+
----- SOURCE CODE
6+
pub fn main() { let _ = -----7 }
7+
8+
----- WARNING
9+
warning: Unnecessary double negation (--) on integer
10+
┌─ /src/warning/wrn.gleam:1:25
11+
12+
1pub fn main() { let _ = -----7 }
13+
^^^^ You can safely remove this.

compiler-core/src/type_/tests/warnings.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,28 @@ fn double_unary_integer_literal() {
464464
assert_warning!("pub fn main() { let _ = --7 }");
465465
}
466466

467+
#[test]
468+
fn even_number_of_multiple_integer_negations_raise_a_single_warning() {
469+
assert_warning!("pub fn main() { let _ = ----7 }");
470+
}
471+
472+
#[test]
473+
fn odd_number_of_multiple_integer_negations_raise_a_single_warning_that_highlights_the_unnecessary_ones()
474+
{
475+
assert_warning!("pub fn main() { let _ = -----7 }");
476+
}
477+
478+
#[test]
479+
fn even_number_of_multiple_bool_negations_raise_a_single_warning() {
480+
assert_warning!("pub fn main() { let _ = !!!!True }");
481+
}
482+
483+
#[test]
484+
fn odd_number_of_multiple_bool_negations_raise_a_single_warning_that_highlights_the_unnecessary_ones()
485+
{
486+
assert_warning!("pub fn main() { let _ = !!!!!False }");
487+
}
488+
467489
// https://github.com/gleam-lang/gleam/issues/2050
468490
#[test]
469491
fn double_unary_integer_variable() {

0 commit comments

Comments
 (0)