-
Notifications
You must be signed in to change notification settings - Fork 154
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
merge branch in if #166
base: master
Are you sure you want to change the base?
merge branch in if #166
Conversation
✅ Deploy Preview for webcrack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
How about an additional flip-conditionals transform instead of modifying merge-else-if?
Would solve your original issue and a few others at the same time nvm just tried this and it made some improvements but also resulted in similarly unreadable code like in the webpack test |
Done, but it still affects the webpack test because it's an
IMO this should be solved by another transformation which transforms things like |
Fixed the unreadable webpack code by preventing the merge if it's already an else if |
Already exists: https://webcrack.netlify.app/docs/concepts/unminify.html#invert-boolean-logic |
Hmm not sure how to go about that... It successfully flattened it but due to the order of transforms (onExit, meaning merge-else-if will run after invert-boolean-logic) there's an extra And in other cases getting rid of
|
There's more such cases, e.g. I have to re-run webcrack 4 times before it becomes stable. We can explicitly call the invert-boolean-logic transform here like how control-flow-object is explicitly calling merge-strings. webcrack/packages/webcrack/src/deobfuscate/control-flow-object.ts Lines 187 to 191 in 16a833c
|
yes that's technically possible but have to be careful to avoid quadratic runtime (traverse AST while traversing AST) or infinite loops |
Yeah that's what I meant, will push a commit for this in a bit. Update: looks like it doesn't work for some reason. The test is failing but it shouldn't 🤷 diff --git a/packages/webcrack/src/unminify/test/merge-else-if.test.ts b/packages/webcrack/src/unminify/test/merge-else-if.test.ts
index 1a11585..887fa67 100644
--- a/packages/webcrack/src/unminify/test/merge-else-if.test.ts
+++ b/packages/webcrack/src/unminify/test/merge-else-if.test.ts
@@ -21,7 +21,7 @@ test('merge', () => {
} else {
console.log("branch 1");
}`).toMatchInlineSnapshot(`
- if (!!cond) {
+ if (cond) {
console.log("branch 1");
} else if (cond2) {
console.log("branch 2");
diff --git a/packages/webcrack/src/unminify/transforms/merge-else-if.ts b/packages/webcrack/src/unminify/transforms/merge-else-if.ts
index c8d3b19..bfa76cf 100644
--- a/packages/webcrack/src/unminify/transforms/merge-else-if.ts
+++ b/packages/webcrack/src/unminify/transforms/merge-else-if.ts
@@ -1,6 +1,7 @@
import * as m from '@codemod/matchers';
import * as t from '@babel/types';
-import type { Transform } from '../../ast-utils';
+import { applyTransform, type Transform } from '../../ast-utils';
+import invertBooleanLogic from './invert-boolean-logic';
export default {
name: 'merge-else-if',
@@ -43,6 +44,10 @@ export default {
path.node.test = t.unaryExpression('!', path.node.test);
path.node.consequent = path.node.alternate!;
path.node.alternate = nestedIf.current;
+ this.changes += applyTransform(
+ path.node.test,
+ invertBooleanLogic,
+ ).changes;
this.changes++;
}
|
closes #165
not sure how I feel on the effect it had on the webpack test