Skip to content
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

Added hint when using function as procedure and vice versa #322

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

anderson4j
Copy link
Collaborator

Will yield errors like:
CALL abs(-50) YIELD * -> Procedure abs is not present in the database. Did you mean to call the function abs? Only procedures can be called inside a CALL clause.
and
RETURN apoc.create.uuids(50) -> Function apoc.create.uuids is not present in the database. Did you mean to call the procedure apoc.create.uuids? Procedures must be called inside a CALL clause.

Instead of the regular "Procedure/Function is not present in the database"-messages.

@anderson4j anderson4j added the auto completion Issues related to the auto-completion label Jan 8, 2025
@anderson4j anderson4j requested a review from ncordon January 8, 2025 14:08
Copy link

changeset-bot bot commented Jan 8, 2025

🦋 Changeset detected

Latest commit: aee377b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@neo4j-cypher/language-support Patch
@neo4j-cypher/language-server Patch
@neo4j-cypher/react-codemirror-playground Patch
@neo4j-cypher/react-codemirror Patch
@neo4j-cypher/schema-poller Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -59,6 +59,42 @@ meaning that it expects at least 3 arguments of types NODE, STRING, ANY
]);
});

test('Syntax validation error on procedure used as function returns helpful message', () => {
const query = `RETURN apoc.create.uuids(50)`;
Copy link
Collaborator

@ncordon ncordon Jan 10, 2025

Choose a reason for hiding this comment

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

Conversely, we should add a test to check

RETURN APOC.create.uuids(50)

would just say the function is not in the database, but not that the procedure is (because procedures are case sensitive)

Copy link
Collaborator Author

@anderson4j anderson4j Jan 10, 2025

Choose a reason for hiding this comment

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

Implemented this now, but then got to thinking.. probably any time someone writes strings that match procedure names except for the wrong case, they actually mean the procedure name, so maybe the better solution is to suggest the procedure name with correct case? (so case-insensitive check + case-correct suggest)

Edit: The proposed version here is a little messier than I thought, and I suppose we could (very unlikely) get 2 procedures that are only different in case, which would give ambiguity here, so think ill leave as is

Copy link
Contributor

@daveajrussell daveajrussell left a comment

Choose a reason for hiding this comment

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

Tested out in the codemirror playground - working for me nicely 👍

@anderson4j anderson4j merged commit 7aa9c3a into main Jan 10, 2025
4 checks passed
@anderson4j anderson4j deleted the procForFunc branch January 10, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto completion Issues related to the auto-completion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants