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

feat: ast-based helpers #111

Merged
merged 35 commits into from
Jan 23, 2024
Merged

feat: ast-based helpers #111

merged 35 commits into from
Jan 23, 2024

Conversation

ojeytonwilliams
Copy link
Contributor

@ojeytonwilliams ojeytonwilliams commented Jan 11, 2024 •
edited
Loading

  • feat: add ast helpers + build script
  • build: add python tests

Checklist:

This is mostly a proof of concept. The idea being that the python test runner can import the astHelper string, evaluate it and then import the new helper functions.

freeCodeCamp/freeCodeCamp#53125 contains a simple example of how they can be used.

@socket-security Socket Security
Copy link

socket-security bot commented Jan 11, 2024 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

View full report↗︎

@ojeytonwilliams ojeytonwilliams mentioned this pull request Jan 11, 2024
Merged
4 tasks
@ojeytonwilliams ojeytonwilliams marked this pull request as ready for review January 11, 2024 14:37
@ojeytonwilliams ojeytonwilliams requested a review from a team as a code owner January 11, 2024 14:37
ShaunSHamilton
ShaunSHamilton reviewed Jan 17, 2024
View reviewed changes
lib/index.ts Outdated Show resolved Hide resolved
@ojeytonwilliams
feat: add ast helpers + build script
f7aff4c
@ojeytonwilliams
build: add python tests
9592fce
@ojeytonwilliams
feat: use chainable class for helpers
ecefda9 
This makes for a much cleaner api
@ojeytonwilliams
feat: add find_class
142868d
@ojeytonwilliams
chore: organise tests into suites
f7eb116
@ojeytonwilliams
feat: add get and find for variables
a22f3b9 
Allowing for easy confirmation that a given variable is assigned a
particular constant value
@ojeytonwilliams
feat: add value_is_call helper
796b363
@ojeytonwilliams
feat: len, find_ifs and find_nth
4c3ddf8
@ojeytonwilliams
feat: make Chainable subscriptable
cac8d78
@ojeytonwilliams
feat: compare ast to string in is_equivalent
30c3aad
@ojeytonwilliams
refactor: wrap tree in module for comparison
be3e3fd
@ojeytonwilliams
feat: compare constants
2f951f3
@ojeytonwilliams
refactor: show that it can handle all expressions
6bb5bdb 
Not just Constants, as I thought.
@ojeytonwilliams
fix: TypeError if is_equivalent is called on None
fa7d897
@ojeytonwilliams
refactor: simplify is_equivalent
26e7095
@ojeytonwilliams
feat: return chainables list, not single chainable
a2ba7d2
@ojeytonwilliams
feat: add find_if_bodies helper
927b233
@ojeytonwilliams
fix: handle printing of Chainable(None)
f9cf98d
@ojeytonwilliams
chore: remove unused functions
00650ae
@ojeytonwilliams
fix: use Chainable(None) if variable not found
2afde0b
@ojeytonwilliams
feat: use is_integer after find_variable
9a461a9 
Separates finding the variable from checking the type
@ojeytonwilliams
feat: change Chainable(None) handling
efd35b8
@ojeytonwilliams
feat: add __eq__
e06e182
@ojeytonwilliams
eat: return Chainable() instead of None
5eed0b5
@ojeytonwilliams
fix: handle unhappy path for find_function
46fc16d
@socket-security Socket Security
Copy link

socket-security bot commented Jan 22, 2024 •
edited
Loading

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

ShaunSHamilton
ShaunSHamilton previously approved these changes Jan 23, 2024
View reviewed changes
Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

We just won't talk about:
image

Also, I assume in another PR we would want to add python/*.py to .gitignore?

What is the point of adding the mock-script for Jest? Why not just ignore everything in python/ when running the tests?

@ojeytonwilliams
Copy link
Contributor Author

Also, I assume in another PR we would want to add python/*.py to .gitignore?

Why? It's source code.

What is the point of adding the mock-script for Jest? Why not just ignore everything in python/ when running the tests?

How, though? Jest tries to import python/py_helpers.py and throws an error because it doesn't know how to deal with python code. Is there a way to make Jest not do that?

@ShaunSHamilton
Copy link
Member

Why? It's source code.

Is it not just used in the build, and then imported as a JS string?

@ojeytonwilliams ojeytonwilliams enabled auto-merge (squash) January 23, 2024 10:19
ShaunSHamilton
ShaunSHamilton approved these changes Jan 23, 2024
View reviewed changes
Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

LGTM 👍

At some point, it would be nice to npmignore1 the Python stuffs.

Footnotes

  1. not gitignore 🤦‍♂️ ↩

@ojeytonwilliams ojeytonwilliams merged commit 5ed512a into freeCodeCamp:main Jan 23, 2024
4 checks passed
@ojeytonwilliams ojeytonwilliams deleted the feat/ast-helpers branch January 23, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShaunSHamilton ShaunSHamilton ShaunSHamilton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ojeytonwilliams @ShaunSHamilton