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.

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

Error while renaming a variable. (TypeError: Duplicate declaration) #67

Closed
Anooxy17 opened this issue Aug 28, 2024 · 14 comments · Fixed by #164
Closed

Error while renaming a variable. (TypeError: Duplicate declaration) #67

Anooxy17 opened this issue Aug 28, 2024 · 14 comments · Fixed by #164

Comments

@Anooxy17
Copy link

Anooxy17 commented Aug 28, 2024

Console output:

[2024-08-28 21:12:49]  Renaming global
[2024-08-28 21:12:49]  Context:     throw CustomError.C(null);
      }
      return {
        us: ______________inputString[7],
        Cp: b
      };
    }
  }
  class Image {
    constructor() {
      this.Sg = this.Tg = this.ya = null;

[2024-08-28 21:12:49]  Renamed to inputString
[2024-08-28 21:12:49]  Processing: 55%
[2024-08-28 21:12:49]  Processing: 100%

C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\core\lib\transformation\file\file.js:195
    return new _Error(msg);
           ^

TypeError: Duplicate declaration "Ya"

    at File.buildCodeFrameError (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\core\lib\transformation\file\file.js:195:12)
    at Scope.checkBlockScopedCollisions (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\scope\index.js:408:22)
    at Scope.registerBinding (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\scope\index.js:544:16)
    at Scope.registerDeclaration (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\scope\index.js:500:12)
    at Object.BlockScoped (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\scope\index.js:259:12)
    at Object.newFn (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\visitors.js:207:17)
    at NodePath._call (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:49:20)
    at NodePath.call (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:35:15)
    at NodePath.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:85:31)
    at TraversalContext.visitQueue (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:89:16)
    at TraversalContext.visitMultiple (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:61:17)
    at TraversalContext.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:110:19)
    at traverseNode (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\traverse-node.js:22:17)
    at NodePath.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:91:52)
    at TraversalContext.visitQueue (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:89:16)
    at TraversalContext.visitSingle (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:65:19)
    at TraversalContext.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:112:19)
    at traverseNode (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\traverse-node.js:22:17)
    at NodePath.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:91:52)
    at TraversalContext.visitQueue (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:89:16)
    at TraversalContext.visitSingle (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:65:19)
    at TraversalContext.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:112:19)
    at traverseNode (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\traverse-node.js:22:17)
    at NodePath.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:91:52)
    at TraversalContext.visitQueue (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:89:16)
    at TraversalContext.visitSingle (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:65:19)
    at TraversalContext.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:112:19)
    at traverseNode (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\traverse-node.js:22:17)
    at NodePath.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:91:52)
    at TraversalContext.visitQueue (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:89:16)
    at TraversalContext.visitMultiple (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:61:17)
    at TraversalContext.visit (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\context.js:110:19)
    at traverseNode (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\traverse-node.js:22:17)
    at traverse (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\index.js:53:34)
    at NodePath.traverse (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\index.js:119:24)
    at Scope.crawl (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\scope\index.js:704:10)
    at Scope.init (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\scope\index.js:675:12)
    at NodePath.setScope (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:123:53)
    at NodePath.setContext (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\traverse\lib\path\context.js:135:8)
    at new File (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\core\lib\transformation\file\file.js:90:8)
    at normalizeFile (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\core\lib\transformation\normalize-file.js:98:10)
    at normalizeFile.next (<anonymous>)
    at run (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\core\lib\transformation\index.js:21:50)
    at run.next (<anonymous>)
    at C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\@babel\core\lib\transform-ast.js:23:33
    at Generator.next (<anonymous>)
    at step (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\gensync\index.js:261:32)
    at C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\gensync\index.js:273:13
    at async.call.result.err.err (C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\gensync\index.js:223:11)
    at C:\Users\ktepe\AppData\Roaming\npm\node_modules\humanifyjs\node_modules\gensync\index.js:50:45
@brianjenkins94
Copy link

brianjenkins94 commented Oct 18, 2024

Is there a workaround or anything for this? Or is this blocked by the effort to understand how to perform the rename without collision?

I would be totally okay if it just became variable$1.

@jehna
Copy link
Owner

jehna commented Oct 18, 2024

@brianjenkins94 did you try with v2.1.3 yet? It should have a better logic against invalid identifiers

@jehna
Copy link
Owner

jehna commented Oct 18, 2024

If the identifier conflicts with another identifier that's not generated by Humanify, then there's no valid fix yet.

I wonder what should be done in case of an identifier that we cannot rename? A quick solution would be to prefix it with an underscore

@jehna
Copy link
Owner

jehna commented Oct 18, 2024

I suppose the best solution could be that Humanify would retry with a prompt like:

Rename a function ..., here is a list of names that you cannot use: ${failedIdentifiers}

@brianjenkins94
Copy link

brianjenkins94 commented Oct 18, 2024

Yes, I am using humanify v2.1.3 with:

  "overrides": {
    "humanifyjs": {
      "webcrack": "latest"
    }
  }

which resolves to webcrack v2.14.1.

If the identifier conflicts with another identifier that's not generated by Humanify, then there's no valid fix yet.

I don't think that's the problem I'm having, all my variables are named v1, p1, etc.

I wonder what should be done in case of an identifier that we cannot rename? A quick solution would be to prefix it with an underscore

That's what I was saying, if it could just add a $1 suffix, like esbuild does to avoid collisions.

here's my repo:

https://github.com/brianjenkins94/reverse-engineered-nodebox

@jehna
Copy link
Owner

jehna commented Oct 18, 2024

Okay, I think I found the issue. Fixing this now

@brianjenkins94
Copy link

I updated my test harness so I can test against the main branch. I'll see if it gets further this time.

Aside: Can I run humanify against multiple files simultaneously? Or would that run the risk of making requests too fast?

@jehna
Copy link
Owner

jehna commented Oct 18, 2024

Just released v2.2.0, should fix this issue for now.

Can I run humanify against multiple files simultaneously? Or would that run the risk of making requests too fast?

I'd say it depends solely on your OpenAI subscription level. Unfortunately there's not that good rate limit throttling/retry logic built into Humanify yet.

@jehna
Copy link
Owner

jehna commented Oct 18, 2024

Added some thoughts about parallelism to issue #167

@0xdevalias
Copy link

Unfortunately there's not that good rate limit throttling/retry logic built into Humanify yet.

Added some thoughts about parallelism to issue #167

See also:

@0xdevalias
Copy link

I suppose the best solution could be that Humanify would retry with a prompt like:

Rename a function ..., here is a list of names that you cannot use: ${failedIdentifiers}

@jehna That's what's been proposed in the past, eg.

Also it can happen that a variable gets renamed but another one with the same name already exists in the scope, or the code gets split up in such a way that not all references are renamed.

Off the top of my head, if it's not easily solved via tweaking the prompt, you could potentially have it do a 'refinement/feedback round' where you provide feedback to the LLM + get it to correct the issues (which could be repeated up to X times) eg.

The following variables were already defined in the scope, so we can't use the names you chose. Please choose a new name for them so that they don't clash:

  • X
  • Y
  • Z

Originally posted by @0xdevalias in #8 (comment)

#164 feels a bit like a workaround rather than an optimal solution.

@0xdevalias
Copy link

Related context (from secondary comment on another issue):

Also it can happen that a variable gets renamed but another one with the same name already exists in the scope, or the code gets split up in such a way that not all references are renamed.
Maybe use babel's scope-aware rename that also handles these conflicts (have to find a way of getting rid of the _ prefix): astexplorer.net#/gist/7283141e13dab314521744603a95e9b7/05b11370db8d5ef257550b2916b87d6e72e4eb1d

Originally posted by @j4k0xb in #8

@brianjenkins94
Copy link

brianjenkins94 commented Oct 21, 2024

Aside: Can I run humanify against multiple files simultaneously? Or would that run the risk of making requests too fast?

In trying to produce benchmark results for #172, I have determined that simultaneous runs cause 429s and cause the application to crash.

Just closing the loop.

This was referenced Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants