Skip to content

Commit

Permalink
Merge pull request #2812 from ruby/update-when-message
Browse files Browse the repository at this point in the history 
Update duplicated when error message
  • Loading branch information
@kddnewton
kddnewton committed May 24, 2024
2 parents ca62c3f + 54316fd commit b863f9a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 15 deletions.
3 changes: 2 additions & 1 deletion include/prism/static_literals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ typedef struct {
* @param start_line The line number that the parser starts on.
* @param literals The set of static literals to add the node to.
* @param node The node to add to the set.
* @param replace Whether to replace the previous node if one already exists.
* @return A pointer to the node that is being overwritten, if there is one.
*/
pm_node_t * pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line, pm_static_literals_t *literals, pm_node_t *node);
pm_node_t * pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line, pm_static_literals_t *literals, pm_node_t *node, bool replace);

/**
* Free the internal memory associated with the given static literals set.
Expand Down
13 changes: 9 additions & 4 deletions src/prism.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13737,7 +13737,7 @@ parse_statements(pm_parser_t *parser, pm_context_t context) {
*/
static void
pm_hash_key_static_literals_add(pm_parser_t *parser, pm_static_literals_t *literals, pm_node_t *node) {
const pm_node_t *duplicated = pm_static_literals_add(&parser->newline_list, parser->start_line, literals, node);
const pm_node_t *duplicated = pm_static_literals_add(&parser->newline_list, parser->start_line, literals, node, true);

if (duplicated != NULL) {
pm_buffer_t buffer = { 0 };
Expand All @@ -13763,13 +13763,16 @@ pm_hash_key_static_literals_add(pm_parser_t *parser, pm_static_literals_t *liter
*/
static void
pm_when_clause_static_literals_add(pm_parser_t *parser, pm_static_literals_t *literals, pm_node_t *node) {
if (pm_static_literals_add(&parser->newline_list, parser->start_line, literals, node) != NULL) {
pm_node_t *previous;

if ((previous = pm_static_literals_add(&parser->newline_list, parser->start_line, literals, node, false)) != NULL) {
pm_diagnostic_list_append_format(
&parser->warning_list,
node->location.start,
node->location.end,
PM_WARN_DUPLICATED_WHEN_CLAUSE,
pm_newline_list_line_column(&parser->newline_list, node->location.start, parser->start_line).line
pm_newline_list_line_column(&parser->newline_list, node->location.start, parser->start_line).line,
pm_newline_list_line_column(&parser->newline_list, previous->location.start, parser->start_line).line
);
}
}
Expand Down Expand Up @@ -16370,7 +16373,7 @@ parse_pattern_hash_implicit_value(pm_parser_t *parser, pm_constant_id_list_t *ca
*/
static void
parse_pattern_hash_key(pm_parser_t *parser, pm_static_literals_t *keys, pm_node_t *node) {
if (pm_static_literals_add(&parser->newline_list, parser->start_line, keys, node) != NULL) {
if (pm_static_literals_add(&parser->newline_list, parser->start_line, keys, node, true) != NULL) {
pm_parser_err_node(parser, node, PM_ERR_PATTERN_HASH_KEY_DUPLICATE);
}
}
Expand Down Expand Up @@ -18132,6 +18135,8 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
// as frozen because when clause strings are frozen.
if (PM_NODE_TYPE_P(condition, PM_STRING_NODE)) {
pm_node_flag_set(condition, PM_STRING_FLAGS_FROZEN | PM_NODE_FLAG_STATIC_LITERAL);
} else if (PM_NODE_TYPE_P(condition, PM_SOURCE_FILE_NODE)) {
pm_node_flag_set(condition, PM_NODE_FLAG_STATIC_LITERAL);
}

pm_when_clause_static_literals_add(parser, &literals, condition);
Expand Down
27 changes: 19 additions & 8 deletions src/static_literals.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ node_hash(const pm_static_literals_metadata_t *metadata, const pm_node_t *node)
* and must be able to compare all node types that will be stored in this hash.
*/
static pm_node_t *
pm_node_hash_insert(pm_node_hash_t *hash, const pm_static_literals_metadata_t *metadata, pm_node_t *node, int (*compare)(const pm_static_literals_metadata_t *metadata, const pm_node_t *left, const pm_node_t *right)) {
pm_node_hash_insert(pm_node_hash_t *hash, const pm_static_literals_metadata_t *metadata, pm_node_t *node, bool replace, int (*compare)(const pm_static_literals_metadata_t *metadata, const pm_node_t *left, const pm_node_t *right)) {
// If we are out of space, we need to resize the hash. This will cause all
// of the nodes to be rehashed and reinserted into the new hash.
if (hash->size * 2 >= hash->capacity) {
Expand Down Expand Up @@ -202,9 +202,14 @@ pm_node_hash_insert(pm_node_hash_t *hash, const pm_static_literals_metadata_t *m
// already in the hash. Otherwise, we can just increment the size and insert
// the new node.
pm_node_t *result = hash->nodes[index];
if (result == NULL) hash->size++;

hash->nodes[index] = node;
if (result == NULL) {
hash->size++;
hash->nodes[index] = node;
} else if (replace) {
hash->nodes[index] = node;
}

return result;
}

Expand Down Expand Up @@ -348,7 +353,7 @@ pm_compare_regular_expression_nodes(PRISM_ATTRIBUTE_UNUSED const pm_static_liter
* Add a node to the set of static literals.
*/
pm_node_t *
pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line, pm_static_literals_t *literals, pm_node_t *node) {
pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line, pm_static_literals_t *literals, pm_node_t *node, bool replace) {
switch (PM_NODE_TYPE(node)) {
case PM_INTEGER_NODE:
case PM_SOURCE_LINE_NODE:
Expand All @@ -360,6 +365,7 @@ pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line
.encoding_name = NULL
},
node,
replace,
pm_compare_integer_nodes
);
case PM_FLOAT_NODE:
Expand All @@ -371,6 +377,7 @@ pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line
.encoding_name = NULL
},
node,
replace,
pm_compare_float_nodes
);
case PM_RATIONAL_NODE:
Expand All @@ -383,6 +390,7 @@ pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line
.encoding_name = NULL
},
node,
replace,
pm_compare_number_nodes
);
case PM_STRING_NODE:
Expand All @@ -395,6 +403,7 @@ pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line
.encoding_name = NULL
},
node,
replace,
pm_compare_string_nodes
);
case PM_REGULAR_EXPRESSION_NODE:
Expand All @@ -406,6 +415,7 @@ pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line
.encoding_name = NULL
},
node,
replace,
pm_compare_regular_expression_nodes
);
case PM_SYMBOL_NODE:
Expand All @@ -417,26 +427,27 @@ pm_static_literals_add(const pm_newline_list_t *newline_list, int32_t start_line
.encoding_name = NULL
},
node,
replace,
pm_compare_string_nodes
);
case PM_TRUE_NODE: {
pm_node_t *duplicated = literals->true_node;
literals->true_node = node;
if ((duplicated == NULL) || replace) literals->true_node = node;
return duplicated;
}
case PM_FALSE_NODE: {
pm_node_t *duplicated = literals->false_node;
literals->false_node = node;
if ((duplicated == NULL) || replace) literals->false_node = node;
return duplicated;
}
case PM_NIL_NODE: {
pm_node_t *duplicated = literals->nil_node;
literals->nil_node = node;
if ((duplicated == NULL) || replace) literals->nil_node = node;
return duplicated;
}
case PM_SOURCE_ENCODING_NODE: {
pm_node_t *duplicated = literals->source_encoding_node;
literals->source_encoding_node = node;
if ((duplicated == NULL) || replace) literals->source_encoding_node = node;
return duplicated;
}
default:
Expand Down
2 changes: 1 addition & 1 deletion templates/src/diagnostic.c.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
[PM_WARN_COMPARISON_AFTER_COMPARISON] = { "comparison '%.*s' after comparison", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_DOT_DOT_DOT_EOL] = { "... at EOL, should be parenthesized?", PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_DUPLICATED_HASH_KEY] = { "key %.*s is duplicated and overwritten on line %" PRIi32, PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_DUPLICATED_WHEN_CLAUSE] = { "duplicated 'when' clause with line %" PRIi32 " is ignored", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_DUPLICATED_WHEN_CLAUSE] = { "'when' clause on line %" PRIi32 " duplicates 'when' clause on line %" PRIi32 " and is ignored", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_EQUAL_IN_CONDITIONAL_3_3] = { "found `= literal' in conditional, should be ==", PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_EQUAL_IN_CONDITIONAL] = { "found '= literal' in conditional, should be ==", PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_END_IN_METHOD] = { "END in method; use at_exit", PM_WARNING_LEVEL_DEFAULT },
Expand Down
2 changes: 1 addition & 1 deletion test/prism/warnings_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def test_duplicated_hash_key
end

def test_duplicated_when_clause
assert_warning("case 1; when 1, 1; end", "clause with line")
assert_warning("case 1; when 1, 1; end", "when' clause")
end

def test_float_out_of_range
Expand Down

0 comments on commit b863f9a

Please sign in to comment.