diff options
| -rw-r--r-- | prism/config.yml | 1 | ||||
| -rw-r--r-- | prism/prism.c | 166 | ||||
| -rw-r--r-- | prism/templates/src/diagnostic.c.erb | 3 | ||||
| -rw-r--r-- | test/prism/warnings_test.rb | 72 |
4 files changed, 234 insertions, 8 deletions
diff --git a/prism/config.yml b/prism/config.yml index 6341233f83..889ceccbf4 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -270,6 +270,7 @@ warnings: - SHEBANG_CARRIAGE_RETURN - UNEXPECTED_CARRIAGE_RETURN - UNUSED_LOCAL_VARIABLE + - VOID_STATEMENT tokens: - name: EOF value: 1 diff --git a/prism/prism.c b/prism/prism.c index 93328247e0..3141b2749c 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -1183,6 +1183,158 @@ pm_assert_value_expression(pm_parser_t *parser, pm_node_t *node) { } /** + * Warn if the given node is a "void" statement. + */ +static void +pm_void_statement_check(pm_parser_t *parser, const pm_node_t *node) { + const char *type = NULL; + int length = 0; + + switch (PM_NODE_TYPE(node)) { + case PM_BACK_REFERENCE_READ_NODE: + case PM_CLASS_VARIABLE_READ_NODE: + case PM_GLOBAL_VARIABLE_READ_NODE: + case PM_INSTANCE_VARIABLE_READ_NODE: + case PM_LOCAL_VARIABLE_READ_NODE: + case PM_NUMBERED_REFERENCE_READ_NODE: + type = "a variable"; + length = 10; + break; + case PM_CALL_NODE: { + const pm_call_node_t *cast = (const pm_call_node_t *) node; + if (cast->call_operator_loc.start != NULL || cast->message_loc.start == NULL) break; + + const pm_constant_t *message = pm_constant_pool_id_to_constant(&parser->constant_pool, cast->name); + switch (message->length) { + case 1: + switch (message->start[0]) { + case '+': + case '-': + case '*': + case '/': + case '%': + case '|': + case '^': + case '&': + case '>': + case '<': + type = (const char *) message->start; + length = 1; + break; + } + break; + case 2: + switch (message->start[1]) { + case '=': + if (message->start[0] == '<' || message->start[0] == '>' || message->start[0] == '!' || message->start[0] == '=') { + type = (const char *) message->start; + length = 2; + } + break; + case '@': + if (message->start[0] == '+' || message->start[0] == '-') { + type = (const char *) message->start; + length = 2; + } + break; + case '*': + if (message->start[0] == '*') { + type = (const char *) message->start; + length = 2; + } + break; + } + break; + case 3: + if (memcmp(message->start, "<=>", 3) == 0) { + type = "<=>"; + length = 3; + } + break; + } + + break; + } + case PM_CONSTANT_PATH_NODE: + type = "::"; + length = 2; + break; + case PM_CONSTANT_READ_NODE: + type = "a constant"; + length = 10; + break; + case PM_DEFINED_NODE: + type = "defined?"; + length = 8; + break; + case PM_FALSE_NODE: + type = "false"; + length = 5; + break; + case PM_FLOAT_NODE: + case PM_IMAGINARY_NODE: + case PM_INTEGER_NODE: + case PM_INTERPOLATED_REGULAR_EXPRESSION_NODE: + case PM_INTERPOLATED_STRING_NODE: + case PM_RATIONAL_NODE: + case PM_REGULAR_EXPRESSION_NODE: + case PM_SOURCE_ENCODING_NODE: + case PM_SOURCE_FILE_NODE: + case PM_SOURCE_LINE_NODE: + case PM_STRING_NODE: + case PM_SYMBOL_NODE: + type = "a literal"; + length = 9; + break; + case PM_NIL_NODE: + type = "nil"; + length = 3; + break; + case PM_RANGE_NODE: { + const pm_range_node_t *cast = (const pm_range_node_t *) node; + + if (PM_NODE_FLAG_P(cast, PM_RANGE_FLAGS_EXCLUDE_END)) { + type = "..."; + length = 3; + } else { + type = ".."; + length = 2; + } + + break; + } + case PM_SELF_NODE: + type = "self"; + length = 4; + break; + case PM_TRUE_NODE: + type = "true"; + length = 4; + break; + default: + break; + } + + if (type != NULL) { + PM_PARSER_WARN_NODE_FORMAT(parser, node, PM_WARN_VOID_STATEMENT, length, type); + } +} + +/** + * Warn if any of the statements that are not the last statement in the list are + * a "void" statement. + */ +static void +pm_void_statements_check(pm_parser_t *parser, const pm_statements_node_t *node) { + if (parser->parsing_eval) return; + + assert(node->body.size > 0); + for (size_t index = 0; index < node->body.size - 1; index++) { + pm_void_statement_check(parser, node->body.nodes[index]); + } +} + +/** * When we're handling the predicate of a conditional, we need to know our * context in order to determine the kind of warning we should deliver to the * user. @@ -12911,6 +13063,8 @@ parse_statements(pm_parser_t *parser, pm_context_t context) { } context_pop(parser); + pm_void_statements_check(parser, statements); + return statements; } @@ -16666,6 +16820,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b pop_block_exits(parser, previous_block_exits); pm_node_list_free(¤t_block_exits); + pm_void_statements_check(parser, statements); return (pm_node_t *) pm_parentheses_node_create(parser, &opening, (pm_node_t *) statements, &parser->previous); } case PM_TOKEN_BRACE_LEFT: { @@ -20137,8 +20292,15 @@ parse_program(pm_parser_t *parser) { parser_lex(parser); pm_statements_node_t *statements = parse_statements(parser, PM_CONTEXT_MAIN); - if (!statements) { + + if (statements == NULL) { statements = pm_statements_node_create(parser); + } else if (!parser->parsing_eval) { + // If we have statements, then the top-level statement should be + // explicitly checked as well. We have to do this here because + // everywhere else we check all but the last statement. + assert(statements->body.size > 0); + pm_void_statement_check(parser, statements->body.nodes[statements->body.size - 1]); } pm_constant_id_list_t locals; @@ -20589,7 +20751,7 @@ pm_parse_success_p(const uint8_t *source, size_t size, const char *data) { pm_node_t *node = pm_parse(&parser); pm_node_destroy(&parser, node); - bool result = parser.error_list.size == 0 && parser.warning_list.size == 0; + bool result = parser.error_list.size == 0; pm_parser_free(&parser); pm_options_free(&options); diff --git a/prism/templates/src/diagnostic.c.erb b/prism/templates/src/diagnostic.c.erb index 957df1946c..5044a69c2b 100644 --- a/prism/templates/src/diagnostic.c.erb +++ b/prism/templates/src/diagnostic.c.erb @@ -352,7 +352,8 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = { [PM_WARN_LITERAL_IN_CONDITION_VERBOSE] = { "%sliteral in %s", PM_WARNING_LEVEL_VERBOSE }, [PM_WARN_SHEBANG_CARRIAGE_RETURN] = { "shebang line ending with \\r may cause problems", PM_WARNING_LEVEL_DEFAULT }, [PM_WARN_UNEXPECTED_CARRIAGE_RETURN] = { "encountered \\r in middle of line, treated as a mere space", PM_WARNING_LEVEL_DEFAULT }, - [PM_WARN_UNUSED_LOCAL_VARIABLE] = { "assigned but unused variable - %.*s", PM_WARNING_LEVEL_VERBOSE } + [PM_WARN_UNUSED_LOCAL_VARIABLE] = { "assigned but unused variable - %.*s", PM_WARNING_LEVEL_VERBOSE }, + [PM_WARN_VOID_STATEMENT] = { "possibly useless use of %.*s in void context", PM_WARNING_LEVEL_VERBOSE } }; /** diff --git a/test/prism/warnings_test.rb b/test/prism/warnings_test.rb index 5f1e746b52..ff9c306c99 100644 --- a/test/prism/warnings_test.rb +++ b/test/prism/warnings_test.rb @@ -24,15 +24,15 @@ module Prism end def test_equal_in_conditional - assert_warning("if a = 1; end; a", "should be ==") + assert_warning("if a = 1; end; a = a", "should be ==") end def test_dot_dot_dot_eol - assert_warning("foo...", "... at EOL") + assert_warning("_ = foo...", "... at EOL") assert_warning("def foo(...) = bar ...", "... at EOL") - assert_warning("foo... #", "... at EOL") - assert_warning("foo... \t\v\f\n", "... at EOL") + assert_warning("_ = foo... #", "... at EOL") + assert_warning("_ = foo... \t\v\f\n", "... at EOL") refute_warning("p foo...bar") refute_warning("p foo... bar") @@ -51,7 +51,7 @@ module Prism end def test_float_out_of_range - assert_warning("1.0e100000", "out of range") + assert_warning("_ = 1.0e100000", "out of range") end def test_integer_in_flip_flop @@ -125,6 +125,68 @@ module Prism refute_warning("def foo; bar = 1; tap { baz = bar; baz }; end") end + def test_void_statements + assert_warning("foo = 1; foo", "a variable in void") + assert_warning("@foo", "a variable in void") + assert_warning("@@foo", "a variable in void") + assert_warning("$foo", "a variable in void") + assert_warning("$+", "a variable in void") + assert_warning("$1", "a variable in void") + + assert_warning("self", "self in void") + assert_warning("nil", "nil in void") + assert_warning("true", "true in void") + assert_warning("false", "false in void") + + assert_warning("1", "literal in void") + assert_warning("1.0", "literal in void") + assert_warning("1r", "literal in void") + assert_warning("1i", "literal in void") + assert_warning(":foo", "literal in void") + assert_warning("\"foo\"", "literal in void") + assert_warning("\"foo\#{1}\"", "literal in void") + assert_warning("/foo/", "literal in void") + assert_warning("/foo\#{1}/", "literal in void") + + assert_warning("Foo", "constant in void") + assert_warning("::Foo", ":: in void") + assert_warning("Foo::Bar", ":: in void") + + assert_warning("1..2", ".. in void") + assert_warning("1..", ".. in void") + assert_warning("..2", ".. in void") + assert_warning("1...2", "... in void") + assert_warning("1...;", "... in void") + assert_warning("...2", "... in void") + + assert_warning("defined?(foo)", "defined? in void") + + assert_warning("1 + 1", "+ in void") + assert_warning("1 - 1", "- in void") + assert_warning("1 * 1", "* in void") + assert_warning("1 / 1", "/ in void") + assert_warning("1 % 1", "% in void") + assert_warning("1 | 1", "| in void") + assert_warning("1 ^ 1", "^ in void") + assert_warning("1 & 1", "& in void") + assert_warning("1 > 1", "> in void") + assert_warning("1 < 1", "< in void") + + assert_warning("1 ** 1", "** in void") + assert_warning("1 <= 1", "<= in void") + assert_warning("1 >= 1", ">= in void") + assert_warning("1 != 1", "!= in void") + assert_warning("1 == 1", "== in void") + assert_warning("1 <=> 1", "<=> in void") + + assert_warning("+foo", "+@ in void") + assert_warning("-foo", "-@ in void") + + assert_warning("def foo; @bar; @baz; end", "variable in void") + refute_warning("def foo; @bar; end") + refute_warning("@foo", compare: false, scopes: [[]]) + end + private def assert_warning(source, message) |