Skip to content

Improve global variable alias error messaging #3610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

allcre
Copy link

@allcre allcre commented Jul 22, 2025

Improve error message for invalid global variable aliasing

Summary

This PR improves the error message when attempting to alias a global variable with a non-global variable. The previous generic error message was misleading and has been replaced with a more specific and descriptive one.

The Problem

The old error message was confusing because it didn't explain the actual constraint. Consider this example:

alias $a b
         ^ invalid argument being passed to `alias`; expected a bare word, symbol, constant, or global variable

This error message is misleading because:

  1. b is a bare word, which the error says is expected
  2. The message doesn't explain the actual rule: when aliasing global variables, both names must be global variables

The Solution

The new error message clearly states the actual requirement:

alias $a b
         ^ invalid argument being passed to `alias`; when aliasing global variables, both the old and new names must be global variables

Changes

  • Added new error constant PM_ERR_ALIAS_ARGUMENT_GLOBAL_VARIABLE to provide clearer error messaging
  • Updated the parser to use the new error when aliasing global variables with invalid arguments
  • Updated test case aliasing_global_variable_with_non_global_variable.txt to verify the new error message

@allcre allcre force-pushed the ac-improve_global_variable_alias_error_messaging branch from 028e183 to 59868a0 Compare July 22, 2025 18:44
@allcre allcre force-pushed the ac-improve_global_variable_alias_error_messaging branch from 59868a0 to e55dde8 Compare July 22, 2025 18:45
@@ -86,6 +86,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {

// Errors that should raise syntax errors
[PM_ERR_ALIAS_ARGUMENT] = { "invalid argument being passed to `alias`; expected a bare word, symbol, constant, or global variable", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_ALIAS_ARGUMENT_GLOBAL_VARIABLE] = { "invalid argument being passed to `alias`; when aliasing global variables, both the old and new names must be global variables", PM_ERROR_LEVEL_SYNTAX },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the message can be more specific. Since this error only happens when the first (new) name is already a global variable, we can probably say

invalid argument being passed to `alias`; expected the old name to be a global variable as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old name might be unclear if it's the first or second argument (alias is notoriously confusing about this), so I'd suggest to use the second name instead of the old name, or the message in the current state of this PR (which sounds clear to me).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. My point is mostly that the message can be more concise. the second name indeed works better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants