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.

Sign up for GitHub

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

Jump to bottom

HCL Expression AST #6178

Merged
merged 1 commit into from
Sep 23, 2023
Merged

HCL Expression AST #6178

merged 1 commit into from
Sep 23, 2023

Conversation

lkishalmi
Copy link
Contributor

This one adds the AST for the HCL Expression language. That would be required for a more in-depth semantic analyzer, formatter, hints, whatever.

@lkishalmi lkishalmi added the HCL label Jul 11, 2023
@mbien mbien added Upgrade JDK Upgrade to the JDK requirements of a module. ci:all-tests [ci] enable all tests labels Jul 11, 2023
vieiro
vieiro approved these changes Jul 16, 2023
View reviewed changes
Copy link
Contributor

@vieiro vieiro left a comment

Choose a reason for hiding this comment

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

Wow, this is looking great. I've left some comments you may want to review. Good job!

ide/languages.hcl/manifest.mf Show resolved Hide resolved
...anguages.hcl/src/org/netbeans/modules/languages/hcl/ast/expression/HCLExpressionFactory.java Outdated Show resolved Hide resolved
@lkishalmi
Copy link
Contributor Author

I would call for more review and approval once this PR would be ready. I've raised this as a preview to get help from @mbien resolbing my Java 11 vs UnitTest issues.

I would have liked this included in NetBeans 19. Though it is not ready yet, there are lot of crufts to be removed/polished, also this alone does not add any functionality to the IDE at the moment, Though many advanced IDE things requires to be able to have the complete AST available..

@lkishalmi lkishalmi added this to the NB20 milestone Jul 18, 2023
@lkishalmi lkishalmi force-pushed the languages.hcl-ast-expression branch from 9da619d to 8fd5582 Compare July 27, 2023 00:48
@lkishalmi lkishalmi marked this pull request as ready for review July 27, 2023 00:48
@lkishalmi
Copy link
Contributor Author

image

Having better AST, the Semantic analyzer recognize, functions, object keys as well.

The coloring are not final, though it's time to move on the indexing, code completion and schema validation....

@mbien
Copy link
Member

mbien commented Aug 21, 2023

I looked through the log and I believe the reason for the failing graal tests is the following:

    [junit] 	org.netbeans.modules.editor.kit - The module org.netbeans.modules.languages.hcl would also need to be installed.

which causes a chain reaction since many other modules do also depend on it. A green run has also many modules failing to load, so what I did was to sort the log, and diff good and bad run, here are the additional modules which fail to load in this PR:

    [junit] 	org.netbeans.modules.ant.kit - The module org.netbeans.modules.ide.kit would also need to be installed.
    [junit] 	org.netbeans.modules.apisupport.kit - The module org.netbeans.modules.java.kit would also need to be installed.
    [junit] 	org.netbeans.modules.db.kit - The module org.netbeans.modules.ide.kit would also need to be installed.
    [junit] 	org.netbeans.modules.editor.kit - The module org.netbeans.modules.languages.hcl would also need to be installed.
    [junit] 	org.netbeans.modules.ide.branding.kit - The module org.netbeans.modules.ide.kit would also need to be installed.
    [junit] 	org.netbeans.modules.ide.kit - The module org.netbeans.modules.editor.kit would also need to be installed.
    [junit] 	org.netbeans.modules.j2ee.kit - The module org.netbeans.modules.web.kit would also need to be installed.
    [junit] 	org.netbeans.modules.j2ee.persistence.kit - The module org.netbeans.modules.db.kit would also need to be installed.
    [junit] 	org.netbeans.modules.j2ee.persistence.kit - The module org.netbeans.modules.java.kit would also need to be installed.
    [junit] 	org.netbeans.modules.java.kit - The module org.netbeans.modules.ant.kit would also need to be installed.
    [junit] 	org.netbeans.modules.languages.hcl - The Java platform version 11 or higher was requested but only 1.8 is running.
    [junit] 	org.netbeans.modules.spring.beans - The module org.netbeans.modules.java.kit would also need to be installed.
    [junit] 	org.netbeans.modules.spring.webmvc - The module named org.netbeans.modules.spring.beans/0-1 was needed and not found.
    [junit] 	org.netbeans.modules.spring.webmvc - The module org.netbeans.modules.web.kit would also need to be installed.
    [junit] 	org.netbeans.modules.timers - The module org.netbeans.modules.ide.kit would also need to be installed.
    [junit] 	org.netbeans.modules.web.jsf.kit - The module org.netbeans.modules.web.kit would also need to be installed.
    [junit] 	org.netbeans.modules.web.kit - The module org.netbeans.modules.j2ee.persistence.kit would also need to be installed.
    [junit] 	org.netbeans.modules.web.kit - The module org.netbeans.modules.java.kit would also need to be installed.
    [junit] 	org.netbeans.modules.web.primefaces - The module org.netbeans.modules.web.jsf.kit would also need to be installed.
    [junit] 	org.netbeans.modules.web.struts - The module org.netbeans.modules.web.kit would also need to be installed.
    [junit] 	org.netbeans.modules.websvc.kit - The module org.netbeans.modules.db.kit would also need to be installed.
    [junit] 	org.netbeans.modules.websvc.kit - The module org.netbeans.modules.java.kit would also need to be installed.
    [junit] 	org.netbeans.modules.websvc.kit - The module org.netbeans.modules.web.kit would also need to be installed.
    [junit] 	org.netbeans.modules.websvc.restkit - The module org.netbeans.modules.websvc.kit would also need to be installed.
    [junit] 	org.netbeans.modules.websvc.restkit - The module org.netbeans.modules.websvc.saas.kit would also need to be installed.
    [junit] 	org.netbeans.modules.websvc.saas.kit - The module org.netbeans.modules.ide.kit would also need to be installed.

It might be time to bump the graal tests to JDK 11 baseline since older CE builds can't even be downloaded from official sources: https://www.graalvm.org/downloads/

The DL url did also change slightly I think. Better to put it into a separate PR.

@lkishalmi
Copy link
Contributor Author

@mbien I can wait for that PR and rebase.

Maybe including HCL support should not be in the base IDE...

Coding all of these I'm eager to move to at least Java 17...

@mbien mbien mentioned this pull request Aug 22, 2023
Closed
@lkishalmi
Copy link
Contributor Author

@mbien any news on the Graal? Moss is growing fat on this PR.

@mbien
Copy link
Member

mbien commented Sep 8, 2023

@lkishalmi #6369 is not ready yet and I had no time to take another look after the initial attempt.

its another case where dependencies where not kept up to date for a while.

@lkishalmi
Copy link
Contributor Author

Rebased on master, squashed. I hope for a green one, then would merge.

@mbien
Copy link
Member

mbien commented Sep 23, 2023

the graal job might need some restarts if tests fail since it is missing the retry script. I am going to add it again in a followup PR.

@lkishalmi lkishalmi merged commit fbdda86 into apache:master Sep 23, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbien mbien mbien left review comments

@vieiro vieiro vieiro approved these changes

Assignees
No one assigned
Labels
ci:all-tests [ci] enable all tests HCL Upgrade JDK Upgrade to the JDK requirements of a module.
Projects
None yet
Milestone
NB20
Development

Successfully merging this pull request may close these issues.
3 participants
@lkishalmi @mbien @vieiro