Much stronger nondeterminism detection #7
Conversation
…defined values that way)
…n that's passed a WorkflowContext
- Move the rules + the tests into their own directories - Write some tests (will switch the testing framework to jest later)
…a TODO. Change some error messages.
| type GlobalTools = {eslintContext: EslintContext, parserServices: ParserServicesWithTypeInformation, typeChecker: TypeChecker}; | ||
| let GLOBAL_TOOLS: GlobalTools | undefined = undefined; | ||
|
|
||
| // These included `Transaction` and `TransactionContext` respectively before! |
There was a problem hiding this comment.
What does this mean? I think we want to add Transaction and TransactionContext in the following lines.
There was a problem hiding this comment.
Yes - Peter told me we should leave them out; they were there before, but keeping them in resulted in lots of false positives on the dbos-demo-apps projects
There was a problem hiding this comment.
Yeah, we should only check workflows for now
There was a problem hiding this comment.
Okay, let's do it in a future PR.
I think your new rule that allows await on a function with a ctxt param would already eliminate most of the false positives.
|
|
||
| try { | ||
| if (Node.isSourceFile(tsMorphNode)) { | ||
| tsMorphNode.getFunctions().forEach(evaluateFunctionForDeterminism); |
There was a problem hiding this comment.
Since functions outside of classes can't have decorators, do we still need this?
There was a problem hiding this comment.
Yes - there might be classes inside of the functions, and the getFunctions and getClasses calls only give you the outermost classes and functions, so you have to recur on the functions and classes inside of all the functions and classes respectively
CaspianA1
force-pushed
the
determinism_rules
branch
from
984cded to
1beea51
Compare
CaspianA1
force-pushed
the
determinism_rules
branch
from
58ce812 to
56ec5ea
Compare
…ounts (this works better for variadic functions, like 'console.log' and 'setTimeout')
ts-morphfrom atypescript-eslintentrypoint