Skip to content

Commit 20ee954

Browse files
yorickpeterseyorickpeterse
committed
Warn for unused method results
The compiler now produces a warning when calling a method for which the return value is unused. Such warnings are added based on a heuristic rather than relying on user-provided annotations, as I'm not a fan of an annotation based approach (= they end up being littered everywhere). The heuristic is simple and as follows: - We only warn when returning _owned_ values - We don't warn when returning an Option, _unless_ the receiver of the call is also Option (e.g. the result of Option.map) - We don't warn when returning Nil or Bool, as these types are pretty much always safe to ignore (e.g. the result of Set.insert) This fixes #809. Changelog: added
1 parent bf15832 commit 20ee954

36 files changed

+346
-126
lines changed

‎compiler/src/diagnostics.rs

+11
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ pub(crate) enum DiagnosticId {
2727
Moved,
2828
Unreachable,
2929
UnusedSymbol,
30+
UnusedResult,
3031
}
3132

3233
impl fmt::Display for DiagnosticId {
@@ -53,6 +54,7 @@ impl fmt::Display for DiagnosticId {
5354
DiagnosticId::MissingMain => "missing-main",
5455
DiagnosticId::InvalidCast => "invalid-cast",
5556
DiagnosticId::UnusedSymbol => "unused-symbol",
57+
DiagnosticId::UnusedResult => "unused-result",
5658
};
5759

5860
write!(f, "{}", id)
@@ -1079,6 +1081,15 @@ impl Diagnostics {
10791081
);
10801082
}
10811083

1084+
pub(crate) fn unused_result(&mut self, file: PathBuf, location: Location) {
1085+
self.warn(
1086+
DiagnosticId::UnusedResult,
1087+
"the result of this expression is unused",
1088+
file,
1089+
location,
1090+
);
1091+
}
1092+
10821093
pub(crate) fn iter(&self) -> impl Iterator<Item = &Diagnostic> {
10831094
self.values.iter()
10841095
}

‎compiler/src/hir.rs

+54-1
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ pub(crate) struct ConstantRef {
151151
pub(crate) struct IdentifierRef {
152152
pub(crate) name: String,
153153
pub(crate) kind: types::IdentifierKind,
154+
/// A flag indicating the result of this node is unused.
155+
pub(crate) unused: bool,
154156
pub(crate) location: Location,
155157
}
156158

@@ -170,6 +172,8 @@ pub(crate) struct Call {
170172
pub(crate) parens: bool,
171173
/// A flag indicating if the call resides directly in a `mut` expression.
172174
pub(crate) in_mut: bool,
175+
/// A flag indicating the result of this node is unused.
176+
pub(crate) unused: bool,
173177
pub(crate) location: Location,
174178
}
175179

@@ -2117,6 +2121,7 @@ impl<'a> LowerToHir<'a> {
21172121
},
21182122
parens: false,
21192123
in_mut: false,
2124+
unused: false,
21202125
arguments: Vec::new(),
21212126
location: loc,
21222127
}));
@@ -2149,6 +2154,7 @@ impl<'a> LowerToHir<'a> {
21492154
let var_ref = Expression::IdentifierRef(Box::new(IdentifierRef {
21502155
name: ARRAY_LIT_VAR.to_string(),
21512156
kind: types::IdentifierKind::Unknown,
2157+
unused: false,
21522158
location: node.location,
21532159
}));
21542160

@@ -2170,6 +2176,7 @@ impl<'a> LowerToHir<'a> {
21702176
},
21712177
parens: true,
21722178
in_mut: false,
2179+
unused: false,
21732180
arguments: vec![Argument::Positional(Box::new(
21742181
PositionalArgument {
21752182
value: arg,
@@ -2208,6 +2215,7 @@ impl<'a> LowerToHir<'a> {
22082215
},
22092216
parens: true,
22102217
in_mut: false,
2218+
unused: false,
22112219
arguments: vec![Argument::Positional(Box::new(
22122220
PositionalArgument {
22132221
value: Expression::Int(Box::new(IntLiteral {
@@ -2310,7 +2318,20 @@ impl<'a> LowerToHir<'a> {
23102318
}
23112319

23122320
fn expressions(&mut self, node: ast::Expressions) -> Vec<Expression> {
2313-
self.values(node.values)
2321+
let mut nodes = self.values(node.values);
2322+
let max = nodes.len().saturating_sub(1);
2323+
2324+
for (idx, node) in nodes.iter_mut().enumerate() {
2325+
let rem = idx < max;
2326+
2327+
match node {
2328+
Expression::Call(c) => c.unused = rem,
2329+
Expression::IdentifierRef(c) => c.unused = rem,
2330+
_ => {}
2331+
}
2332+
}
2333+
2334+
nodes
23142335
}
23152336

23162337
fn values(&mut self, nodes: Vec<ast::Expression>) -> Vec<Expression> {
@@ -2469,6 +2490,7 @@ impl<'a> LowerToHir<'a> {
24692490
},
24702491
parens: true,
24712492
in_mut: false,
2493+
unused: false,
24722494
arguments: vec![Argument::Positional(Box::new(
24732495
PositionalArgument {
24742496
value: self.expression(node.right),
@@ -2502,6 +2524,7 @@ impl<'a> LowerToHir<'a> {
25022524
Box::new(IdentifierRef {
25032525
kind: types::IdentifierKind::Unknown,
25042526
name: node.name,
2527+
unused: false,
25052528
location: node.location,
25062529
})
25072530
}
@@ -2535,6 +2558,7 @@ impl<'a> LowerToHir<'a> {
25352558
name: self.identifier(node.name),
25362559
parens: node.arguments.is_some(),
25372560
in_mut: false,
2561+
unused: false,
25382562
arguments: self.optional_call_arguments(node.arguments),
25392563
location: node.location,
25402564
}))
@@ -2695,6 +2719,7 @@ impl<'a> LowerToHir<'a> {
26952719
let receiver = Expression::IdentifierRef(Box::new(IdentifierRef {
26962720
kind: types::IdentifierKind::Unknown,
26972721
name: variable.name.clone(),
2722+
unused: false,
26982723
location: variable.location,
26992724
}));
27002725

@@ -2710,6 +2735,7 @@ impl<'a> LowerToHir<'a> {
27102735
receiver: Some(receiver),
27112736
parens: true,
27122737
in_mut: false,
2738+
unused: false,
27132739
arguments: vec![Argument::Positional(Box::new(
27142740
PositionalArgument {
27152741
value: self.expression(node.value),
@@ -2748,6 +2774,7 @@ impl<'a> LowerToHir<'a> {
27482774
receiver: Some(receiver),
27492775
parens: true,
27502776
in_mut: false,
2777+
unused: false,
27512778
arguments: vec![Argument::Positional(Box::new(
27522779
PositionalArgument {
27532780
value: self.expression(node.value),
@@ -2801,6 +2828,7 @@ impl<'a> LowerToHir<'a> {
28012828
name: name.clone(),
28022829
parens: false,
28032830
in_mut: false,
2831+
unused: false,
28042832
arguments: Vec::new(),
28052833
location: getter_loc,
28062834
}));
@@ -2814,6 +2842,7 @@ impl<'a> LowerToHir<'a> {
28142842
receiver: Some(getter_rec),
28152843
parens: true,
28162844
in_mut: false,
2845+
unused: false,
28172846
arguments: vec![Argument::Positional(Box::new(
28182847
PositionalArgument {
28192848
value: self.expression(node.value),
@@ -5550,6 +5579,7 @@ mod tests {
55505579
},
55515580
parens: false,
55525581
in_mut: false,
5582+
unused: false,
55535583
arguments: Vec::new(),
55545584
location: cols(12, 13)
55555585
})),
@@ -5599,6 +5629,7 @@ mod tests {
55995629
},
56005630
parens: true,
56015631
in_mut: false,
5632+
unused: false,
56025633
arguments: vec![Argument::Positional(Box::new(
56035634
PositionalArgument {
56045635
value: Expression::Int(Box::new(
@@ -5622,6 +5653,7 @@ mod tests {
56225653
IdentifierRef {
56235654
name: "$array".to_string(),
56245655
kind: types::IdentifierKind::Unknown,
5656+
unused: false,
56255657
location: cols(8, 11),
56265658
}
56275659
))),
@@ -5631,6 +5663,7 @@ mod tests {
56315663
},
56325664
parens: true,
56335665
in_mut: false,
5666+
unused: false,
56345667
arguments: vec![Argument::Positional(Box::new(
56355668
PositionalArgument {
56365669
value: Expression::Int(Box::new(IntLiteral {
@@ -5646,6 +5679,7 @@ mod tests {
56465679
Expression::IdentifierRef(Box::new(IdentifierRef {
56475680
name: "$array".to_string(),
56485681
kind: types::IdentifierKind::Unknown,
5682+
unused: false,
56495683
location: cols(8, 11),
56505684
})),
56515685
],
@@ -5696,6 +5730,7 @@ mod tests {
56965730
},
56975731
parens: true,
56985732
in_mut: false,
5733+
unused: false,
56995734
arguments: vec![Argument::Positional(Box::new(
57005735
PositionalArgument {
57015736
value: Expression::Int(Box::new(
@@ -5716,6 +5751,7 @@ mod tests {
57165751
Expression::IdentifierRef(Box::new(IdentifierRef {
57175752
name: "$array".to_string(),
57185753
kind: types::IdentifierKind::Unknown,
5754+
unused: false,
57195755
location: loc(2, 4, 15, 15),
57205756
})),
57215757
],
@@ -5787,6 +5823,7 @@ mod tests {
57875823
}))),
57885824
parens: true,
57895825
in_mut: false,
5826+
unused: false,
57905827
arguments: vec![Argument::Positional(Box::new(
57915828
PositionalArgument {
57925829
value: Expression::Int(Box::new(IntLiteral {
@@ -5849,6 +5886,7 @@ mod tests {
58495886
IdentifierRef {
58505887
kind: types::IdentifierKind::Unknown,
58515888
name: "a".to_string(),
5889+
unused: false,
58525890
location: cols(8, 8)
58535891
}
58545892
))),
@@ -5858,6 +5896,7 @@ mod tests {
58585896
},
58595897
parens: false,
58605898
in_mut: false,
5899+
unused: false,
58615900
arguments: Vec::new(),
58625901
location: cols(8, 10)
58635902
}))
@@ -5873,6 +5912,7 @@ mod tests {
58735912
Expression::IdentifierRef(Box::new(IdentifierRef {
58745913
kind: types::IdentifierKind::Unknown,
58755914
name: "a".to_string(),
5915+
unused: false,
58765916
location: cols(8, 8)
58775917
}))
58785918
);
@@ -5893,6 +5933,7 @@ mod tests {
58935933
},
58945934
parens: true,
58955935
in_mut: false,
5936+
unused: false,
58965937
arguments: vec![Argument::Positional(Box::new(
58975938
PositionalArgument {
58985939
value: Expression::Int(Box::new(IntLiteral {
@@ -5923,6 +5964,7 @@ mod tests {
59235964
},
59245965
parens: true,
59255966
in_mut: false,
5967+
unused: false,
59265968
arguments: vec![Argument::Named(Box::new(NamedArgument {
59275969
index: 0,
59285970
name: Identifier {
@@ -5954,6 +5996,7 @@ mod tests {
59545996
IdentifierRef {
59555997
kind: types::IdentifierKind::Unknown,
59565998
name: "a".to_string(),
5999+
unused: false,
59576000
location: cols(8, 8)
59586001
}
59596002
))),
@@ -5963,6 +6006,7 @@ mod tests {
59636006
},
59646007
parens: false,
59656008
in_mut: false,
6009+
unused: false,
59666010
arguments: Vec::new(),
59676011
location: cols(8, 10)
59686012
}))
@@ -6134,11 +6178,13 @@ mod tests {
61346178
IdentifierRef {
61356179
kind: types::IdentifierKind::Unknown,
61366180
name: "a".to_string(),
6181+
unused: false,
61376182
location: cols(8, 8)
61386183
}
61396184
))),
61406185
parens: true,
61416186
in_mut: false,
6187+
unused: false,
61426188
arguments: vec![Argument::Positional(Box::new(
61436189
PositionalArgument {
61446190
value: Expression::Int(Box::new(IntLiteral {
@@ -6172,6 +6218,7 @@ mod tests {
61726218
receiver: Expression::IdentifierRef(Box::new(IdentifierRef {
61736219
kind: types::IdentifierKind::Unknown,
61746220
name: "a".to_string(),
6221+
unused: false,
61756222
location: cols(8, 8)
61766223
})),
61776224
name: Identifier {
@@ -6200,6 +6247,7 @@ mod tests {
62006247
receiver: Expression::IdentifierRef(Box::new(IdentifierRef {
62016248
kind: types::IdentifierKind::Unknown,
62026249
name: "a".to_string(),
6250+
unused: false,
62036251
location: cols(8, 8)
62046252
})),
62056253
name: Identifier {
@@ -6214,6 +6262,7 @@ mod tests {
62146262
IdentifierRef {
62156263
kind: types::IdentifierKind::Unknown,
62166264
name: "a".to_string(),
6265+
unused: false,
62176266
location: cols(8, 8)
62186267
}
62196268
))),
@@ -6223,11 +6272,13 @@ mod tests {
62236272
},
62246273
parens: false,
62256274
in_mut: false,
6275+
unused: false,
62266276
arguments: Vec::new(),
62276277
location: cols(8, 10)
62286278
}))),
62296279
parens: true,
62306280
in_mut: false,
6281+
unused: false,
62316282
arguments: vec![Argument::Positional(Box::new(
62326283
PositionalArgument {
62336284
value: Expression::Int(Box::new(IntLiteral {
@@ -6269,6 +6320,7 @@ mod tests {
62696320
}))),
62706321
parens: true,
62716322
in_mut: false,
6323+
unused: false,
62726324
arguments: vec![Argument::Positional(Box::new(
62736325
PositionalArgument {
62746326
value: Expression::Int(Box::new(IntLiteral {
@@ -6659,6 +6711,7 @@ mod tests {
66596711
},
66606712
parens: true,
66616713
in_mut: false,
6714+
unused: false,
66626715
arguments: Vec::new(),
66636716
location: cols(12, 14)
66646717
})),

0 commit comments

Comments
 (0)