-
Notifications
You must be signed in to change notification settings - Fork 2
adding more checkers #2
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
Conversation
|
Thanks a lot for these interesting checkers. |
|
Hi @AryazE, I’ve added tests for all checkers. However, there are 7 checkers I couldn’t get to run successfully:
Let me know if you have any ideas—happy to look into it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds 19 new runtime verification checkers to DyLin, each registered in select_checkers.py and implemented as its own analysis class with a top-of-file spec comment. Key changes:
- Registration of checkers TE-02 through TE-20 in
select_checkers.py - New analyses covering sockets, threading, collections, console I/O, NLTK, random, and HTTP file handling
- Individual
pre_callimplementations enforcing runtime specs (e.g., non-negative timeouts, comparability, file mode)
Reviewed Changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/dylin/select_checkers.py | Adds entries TE-02 to TE-20 for the new checkers |
| src/dylin/analyses/Arrays_Comparable.py | New analysis to check list element comparability before sorting |
| src/dylin/analyses/Sets_Comparable.py | New analysis to check set element comparability before sorting |
| src/dylin/analyses/Console_CloseWriter.py | Warns on close() invoked on sys.stdout |
| src/dylin/analyses/Console_CloseReader.py | Warns on close() invoked on sys.stdin |
| src/dylin/analyses/Console_CloseErrorWriter.py | Warns on close() invoked on sys.stderr |
| src/dylin/analyses/CreateWidgetOnSameFrameCanvas.py | Ensures a widget is added to the same canvas as its frame |
| src/dylin/analyses/HostnamesTerminatesWithSlash.py | Checks that mount URLs terminate with a slash |
| src/dylin/analyses/NLTK_regexp_span_tokenize.py | Verifies the regex passed to regexp_span_tokenize isn’t empty |
| src/dylin/analyses/NLTK_RegexpTokenizerCapturingParentheses.py | Ensures NLTK “RegexpTokenizer” patterns use non-capturing groups |
| src/dylin/analyses/PriorityQueue_NonComparable.py | Flags non-comparable objects being added to a priority queue |
| src/dylin/analyses/PyDocs_MustOnlyAddSynchronizableDataToSharedList.py | Validates that only synchronizable data is appended to shared lists |
| src/dylin/analyses/RandomParams_NoPositives.py | Validates parameters for lognormvariate and vonmisesvariate |
| src/dylin/analyses/RandomRandrange_MustNotUseKwargs.py | Warns against keyword args in random.randrange |
| src/dylin/analyses/Requests_DataMustOpenInBinary.py | Enforces binary mode file opening in requests.api.post |
| src/dylin/analyses/Session_DataMustOpenInBinary.py | Enforces binary mode file opening in Session.post |
| src/dylin/analyses/socket_create_connection.py | Checks non-negative timeout in socket.create_connection |
| src/dylin/analyses/socket_setdefaulttimeout.py | Checks non-negative timeout in socket.setdefaulttimeout |
| src/dylin/analyses/socket_socket_settimeout.py | Checks non-negative timeout in socket.settimeout |
| src/dylin/analyses/Thread_OverrideRun.py | Flags when Thread.run isn’t overridden or no target provided |
Comments suppressed due to low confidence (1)
src/dylin/analyses/RandomParams_NoPositives.py:66
- [nitpick] The error message is generic. Including the specific function name and parameter (e.g., 'sigma <= 0' or 'kappa < 0') will help users quickly identify the violating call.
f"The call to method lognormvariate or vonmisesvariate in file at {dyn_ast} does not have the correct parameters."
| """ | ||
|
|
||
|
|
||
| def is_synchronizable(data): |
Copilot
AI
Jun 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The helper is_synchronizable never returns True for types not explicitly excluded, so valid data is flagged as unsynchronizable. Add a default return True at the end of the function.
| self.analysis_name = "Thread_OverrideRun" | ||
|
|
||
| # Get the hash of the original run method for inspection | ||
| self.original_run_method_hash = sha256(getsource(threading.Thread.run).encode()).hexdigest() |
Copilot
AI
Jun 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling getsource on a built-in or C-implemented method can raise an exception. Consider wrapping this in try/except or using a fallback to avoid runtime errors during analysis initialization.
| self.original_run_method_hash = sha256(getsource(threading.Thread.run).encode()).hexdigest() | |
| try: | |
| source_code = getsource(threading.Thread.run) | |
| self.original_run_method_hash = sha256(source_code.encode()).hexdigest() | |
| except Exception as e: | |
| # Fallback: Log a warning and set a default value | |
| print(f"Warning: Unable to retrieve source code for threading.Thread.run. Exception: {e}") | |
| self.original_run_method_hash = None |
src/dylin/select_checkers.py
Outdated
| "analysis": "dylin.analyses.GradientAnalysis.GradientAnalysis", | ||
| "aliases": ["M-28"], | ||
| }, | ||
| "TE-02": { |
Copilot
AI
Jun 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This block adds many new entries; consider sorting or grouping the checkers logically (e.g., by category) for easier navigation and maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can move most of the checkers to either the "Standard Library" category (SL-*) or the "Programming Constructs" (PC-*). The rest could remain uncategorized until we have something for them.
|
Thanks a lot for the tests, they look very good. Now I have a better understanding of what each checker is looking for. I am a bit unsure about adding the checkers that raise exceptions during runtime. Since DyLin is a dynamic linter, and we need to execute the code to find the issues, the Python interpreter already warns about these errors at the correct location, which makes these checkers redundant. Thanks again for the nice contribution! |
|
I'm also curious about how you came up with these checkers. |
|
Thanks. glad to hear the checkers were helpful! We developed them as part of an ongoing research project and will share more details once the work is published. I’ll address the points you raised above shortly. |
|
Hi @AryazE, I've addressed all the copilot comments and removed all the checkers that python already handles. However, the following are still failing for the following reasons: We're happy to discuss these more if needed |
|
The changes look good to me. |
|
I removed all specs with broken tests, now all tests are passing. |
AryazE
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
Hey there,
We’re working on a research project in runtime verification and have built a set of checkers that we thought might be useful for DyLin. This PR adds 19 new checkers. The description of each checker is available as a comment at the top of its file.
Let me know what you think. I'm Happy to tweak anything if needed!