Skip to content

Commit 802ff71

Browse files
kddnewtonkddnewton
committed
Implement the void statement warning
1 parent 8285032 commit 802ff71

File tree

4 files changed

+234
-8
lines changed

4 files changed

+234
-8
lines changed

‎config.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ warnings:
270270
- SHEBANG_CARRIAGE_RETURN
271271
- UNEXPECTED_CARRIAGE_RETURN
272272
- UNUSED_LOCAL_VARIABLE
273+
- VOID_STATEMENT
273274
tokens:
274275
- name: EOF
275276
value: 1

‎src/prism.c

Lines changed: 164 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,6 +1182,158 @@ pm_assert_value_expression(pm_parser_t *parser, pm_node_t *node) {
11821182
}
11831183
}
11841184

1185+
/**
1186+
* Warn if the given node is a "void" statement.
1187+
*/
1188+
static void
1189+
pm_void_statement_check(pm_parser_t *parser, const pm_node_t *node) {
1190+
const char *type = NULL;
1191+
int length = 0;
1192+
1193+
switch (PM_NODE_TYPE(node)) {
1194+
case PM_BACK_REFERENCE_READ_NODE:
1195+
case PM_CLASS_VARIABLE_READ_NODE:
1196+
case PM_GLOBAL_VARIABLE_READ_NODE:
1197+
case PM_INSTANCE_VARIABLE_READ_NODE:
1198+
case PM_LOCAL_VARIABLE_READ_NODE:
1199+
case PM_NUMBERED_REFERENCE_READ_NODE:
1200+
type = "a variable";
1201+
length = 10;
1202+
break;
1203+
case PM_CALL_NODE: {
1204+
const pm_call_node_t *cast = (const pm_call_node_t *) node;
1205+
if (cast->call_operator_loc.start != NULL || cast->message_loc.start == NULL) break;
1206+
1207+
const pm_constant_t *message = pm_constant_pool_id_to_constant(&parser->constant_pool, cast->name);
1208+
switch (message->length) {
1209+
case 1:
1210+
switch (message->start[0]) {
1211+
case '+':
1212+
case '-':
1213+
case '*':
1214+
case '/':
1215+
case '%':
1216+
case '|':
1217+
case '^':
1218+
case '&':
1219+
case '>':
1220+
case '<':
1221+
type = (const char *) message->start;
1222+
length = 1;
1223+
break;
1224+
}
1225+
break;
1226+
case 2:
1227+
switch (message->start[1]) {
1228+
case '=':
1229+
if (message->start[0] == '<' || message->start[0] == '>' || message->start[0] == '!' || message->start[0] == '=') {
1230+
type = (const char *) message->start;
1231+
length = 2;
1232+
}
1233+
break;
1234+
case '@':
1235+
if (message->start[0] == '+' || message->start[0] == '-') {
1236+
type = (const char *) message->start;
1237+
length = 2;
1238+
}
1239+
break;
1240+
case '*':
1241+
if (message->start[0] == '*') {
1242+
type = (const char *) message->start;
1243+
length = 2;
1244+
}
1245+
break;
1246+
}
1247+
break;
1248+
case 3:
1249+
if (memcmp(message->start, "<=>", 3) == 0) {
1250+
type = "<=>";
1251+
length = 3;
1252+
}
1253+
break;
1254+
}
1255+
1256+
break;
1257+
}
1258+
case PM_CONSTANT_PATH_NODE:
1259+
type = "::";
1260+
length = 2;
1261+
break;
1262+
case PM_CONSTANT_READ_NODE:
1263+
type = "a constant";
1264+
length = 10;
1265+
break;
1266+
case PM_DEFINED_NODE:
1267+
type = "defined?";
1268+
length = 8;
1269+
break;
1270+
case PM_FALSE_NODE:
1271+
type = "false";
1272+
length = 5;
1273+
break;
1274+
case PM_FLOAT_NODE:
1275+
case PM_IMAGINARY_NODE:
1276+
case PM_INTEGER_NODE:
1277+
case PM_INTERPOLATED_REGULAR_EXPRESSION_NODE:
1278+
case PM_INTERPOLATED_STRING_NODE:
1279+
case PM_RATIONAL_NODE:
1280+
case PM_REGULAR_EXPRESSION_NODE:
1281+
case PM_SOURCE_ENCODING_NODE:
1282+
case PM_SOURCE_FILE_NODE:
1283+
case PM_SOURCE_LINE_NODE:
1284+
case PM_STRING_NODE:
1285+
case PM_SYMBOL_NODE:
1286+
type = "a literal";
1287+
length = 9;
1288+
break;
1289+
case PM_NIL_NODE:
1290+
type = "nil";
1291+
length = 3;
1292+
break;
1293+
case PM_RANGE_NODE: {
1294+
const pm_range_node_t *cast = (const pm_range_node_t *) node;
1295+
1296+
if (PM_NODE_FLAG_P(cast, PM_RANGE_FLAGS_EXCLUDE_END)) {
1297+
type = "...";
1298+
length = 3;
1299+
} else {
1300+
type = "..";
1301+
length = 2;
1302+
}
1303+
1304+
break;
1305+
}
1306+
case PM_SELF_NODE:
1307+
type = "self";
1308+
length = 4;
1309+
break;
1310+
case PM_TRUE_NODE:
1311+
type = "true";
1312+
length = 4;
1313+
break;
1314+
default:
1315+
break;
1316+
}
1317+
1318+
if (type != NULL) {
1319+
PM_PARSER_WARN_NODE_FORMAT(parser, node, PM_WARN_VOID_STATEMENT, length, type);
1320+
}
1321+
}
1322+
1323+
/**
1324+
* Warn if any of the statements that are not the last statement in the list are
1325+
* a "void" statement.
1326+
*/
1327+
static void
1328+
pm_void_statements_check(pm_parser_t *parser, const pm_statements_node_t *node) {
1329+
if (parser->parsing_eval) return;
1330+
1331+
assert(node->body.size > 0);
1332+
for (size_t index = 0; index < node->body.size - 1; index++) {
1333+
pm_void_statement_check(parser, node->body.nodes[index]);
1334+
}
1335+
}
1336+
11851337
/**
11861338
* When we're handling the predicate of a conditional, we need to know our
11871339
* context in order to determine the kind of warning we should deliver to the
@@ -12911,6 +13063,8 @@ parse_statements(pm_parser_t *parser, pm_context_t context) {
1291113063
}
1291213064

1291313065
context_pop(parser);
13066+
pm_void_statements_check(parser, statements);
13067+
1291413068
return statements;
1291513069
}
1291613070

@@ -16666,6 +16820,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
1666616820
pop_block_exits(parser, previous_block_exits);
1666716821
pm_node_list_free(&current_block_exits);
1666816822

16823+
pm_void_statements_check(parser, statements);
1666916824
return (pm_node_t *) pm_parentheses_node_create(parser, &opening, (pm_node_t *) statements, &parser->previous);
1667016825
}
1667116826
case PM_TOKEN_BRACE_LEFT: {
@@ -20137,8 +20292,15 @@ parse_program(pm_parser_t *parser) {
2013720292

2013820293
parser_lex(parser);
2013920294
pm_statements_node_t *statements = parse_statements(parser, PM_CONTEXT_MAIN);
20140-
if (!statements) {
20295+
20296+
if (statements == NULL) {
2014120297
statements = pm_statements_node_create(parser);
20298+
} else if (!parser->parsing_eval) {
20299+
// If we have statements, then the top-level statement should be
20300+
// explicitly checked as well. We have to do this here because
20301+
// everywhere else we check all but the last statement.
20302+
assert(statements->body.size > 0);
20303+
pm_void_statement_check(parser, statements->body.nodes[statements->body.size - 1]);
2014220304
}
2014320305

2014420306
pm_constant_id_list_t locals;
@@ -20589,7 +20751,7 @@ pm_parse_success_p(const uint8_t *source, size_t size, const char *data) {
2058920751
pm_node_t *node = pm_parse(&parser);
2059020752
pm_node_destroy(&parser, node);
2059120753

20592-
bool result = parser.error_list.size == 0 && parser.warning_list.size == 0;
20754+
bool result = parser.error_list.size == 0;
2059320755
pm_parser_free(&parser);
2059420756
pm_options_free(&options);
2059520757

‎templates/src/diagnostic.c.erb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,8 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
352352
[PM_WARN_LITERAL_IN_CONDITION_VERBOSE] = { "%sliteral in %s", PM_WARNING_LEVEL_VERBOSE },
353353
[PM_WARN_SHEBANG_CARRIAGE_RETURN] = { "shebang line ending with \\r may cause problems", PM_WARNING_LEVEL_DEFAULT },
354354
[PM_WARN_UNEXPECTED_CARRIAGE_RETURN] = { "encountered \\r in middle of line, treated as a mere space", PM_WARNING_LEVEL_DEFAULT },
355-
[PM_WARN_UNUSED_LOCAL_VARIABLE] = { "assigned but unused variable - %.*s", PM_WARNING_LEVEL_VERBOSE }
355+
[PM_WARN_UNUSED_LOCAL_VARIABLE] = { "assigned but unused variable - %.*s", PM_WARNING_LEVEL_VERBOSE },
356+
[PM_WARN_VOID_STATEMENT] = { "possibly useless use of %.*s in void context", PM_WARNING_LEVEL_VERBOSE }
356357
};
357358

358359
/**

‎test/prism/warnings_test.rb

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@ def test_ambiguous_regexp
2424
end
2525

2626
def test_equal_in_conditional
27-
assert_warning("if a = 1; end; a", "should be ==")
27+
assert_warning("if a = 1; end; a = a", "should be ==")
2828
end
2929

3030
def test_dot_dot_dot_eol
31-
assert_warning("foo...", "... at EOL")
31+
assert_warning("_ = foo...", "... at EOL")
3232
assert_warning("def foo(...) = bar ...", "... at EOL")
3333

34-
assert_warning("foo... #", "... at EOL")
35-
assert_warning("foo... \t\v\f\n", "... at EOL")
34+
assert_warning("_ = foo... #", "... at EOL")
35+
assert_warning("_ = foo... \t\v\f\n", "... at EOL")
3636

3737
refute_warning("p foo...bar")
3838
refute_warning("p foo... bar")
@@ -51,7 +51,7 @@ def test_duplicated_when_clause
5151
end
5252

5353
def test_float_out_of_range
54-
assert_warning("1.0e100000", "out of range")
54+
assert_warning("_ = 1.0e100000", "out of range")
5555
end
5656

5757
def test_integer_in_flip_flop
@@ -125,6 +125,68 @@ def test_unused_local_variables
125125
refute_warning("def foo; bar = 1; tap { baz = bar; baz }; end")
126126
end
127127

128+
def test_void_statements
129+
assert_warning("foo = 1; foo", "a variable in void")
130+
assert_warning("@foo", "a variable in void")
131+
assert_warning("@@foo", "a variable in void")
132+
assert_warning("$foo", "a variable in void")
133+
assert_warning("$+", "a variable in void")
134+
assert_warning("$1", "a variable in void")
135+
136+
assert_warning("self", "self in void")
137+
assert_warning("nil", "nil in void")
138+
assert_warning("true", "true in void")
139+
assert_warning("false", "false in void")
140+
141+
assert_warning("1", "literal in void")
142+
assert_warning("1.0", "literal in void")
143+
assert_warning("1r", "literal in void")
144+
assert_warning("1i", "literal in void")
145+
assert_warning(":foo", "literal in void")
146+
assert_warning("\"foo\"", "literal in void")
147+
assert_warning("\"foo\#{1}\"", "literal in void")
148+
assert_warning("/foo/", "literal in void")
149+
assert_warning("/foo\#{1}/", "literal in void")
150+
151+
assert_warning("Foo", "constant in void")
152+
assert_warning("::Foo", ":: in void")
153+
assert_warning("Foo::Bar", ":: in void")
154+
155+
assert_warning("1..2", ".. in void")
156+
assert_warning("1..", ".. in void")
157+
assert_warning("..2", ".. in void")
158+
assert_warning("1...2", "... in void")
159+
assert_warning("1...;", "... in void")
160+
assert_warning("...2", "... in void")
161+
162+
assert_warning("defined?(foo)", "defined? in void")
163+
164+
assert_warning("1 + 1", "+ in void")
165+
assert_warning("1 - 1", "- in void")
166+
assert_warning("1 * 1", "* in void")
167+
assert_warning("1 / 1", "/ in void")
168+
assert_warning("1 % 1", "% in void")
169+
assert_warning("1 | 1", "| in void")
170+
assert_warning("1 ^ 1", "^ in void")
171+
assert_warning("1 & 1", "& in void")
172+
assert_warning("1 > 1", "> in void")
173+
assert_warning("1 < 1", "< in void")
174+
175+
assert_warning("1 ** 1", "** in void")
176+
assert_warning("1 <= 1", "<= in void")
177+
assert_warning("1 >= 1", ">= in void")
178+
assert_warning("1 != 1", "!= in void")
179+
assert_warning("1 == 1", "== in void")
180+
assert_warning("1 <=> 1", "<=> in void")
181+
182+
assert_warning("+foo", "+@ in void")
183+
assert_warning("-foo", "-@ in void")
184+
185+
assert_warning("def foo; @bar; @baz; end", "variable in void")
186+
refute_warning("def foo; @bar; end")
187+
refute_warning("@foo", compare: false, scopes: [[]])
188+
end
189+
128190
private
129191

130192
def assert_warning(source, message)

0 commit comments

Comments
 (0)