microsoft / microsoft/node-jsonc-parser

getNodePath/modify do not work for property nodes (capture too much content)

Aperta
#51 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Lingua principale
TypeScript
Stelle
759
Fork
66
Merge medio
5g 10h
PR unite (30g)
7

Descrizione

When run on a property node, getNodePath appears to instead returns a path to the enclosing dictionary of the property. This results in some serious problems:

  • findNodeAtLocation(tree, getNodePath(node)) is a different node from node.
  • Calling modify() on a property node removes additional content other than the property node targeted.

Repro steps

Check out the jsonc-parser-demo branch of this github repo.

Run these steps:

npm install && npm run build
node . -d transformBackground testcase.json > testcase-post.json

src/app.ts in this repro is a short script that processes a json file and removes all instances of the property whose key name is given as the -d argument, then prints the file out to STDOUT. To demonstrate the bug more clearly, it currently removes only the first instance before bailing out, and once it has identified the property to be removed it console.warns to STDERR (1) the node it intends to remove (2) the getNodePath of the target node and (3) the roundtrip result of findNodeAtLocation(tree, getNodePath(node)) on the target node. The important code in this example is the code immediately following the comment "PERFORM OPERATIONS HERE"; everything else only serves to load the file and walk the node tree.

My expected behavior running this test script on the included testcase JSON is that on STDERR the "DELETING" and "BUT REALLY DELETING" nodes will be the same (ie, findNodeAtLocation(tree, getNodePath(node)) should be a noop); and on STDOUT the single property "transformBackground" in the dictionary levels/0/screen will be removed.

My observed behavior is quite different:

  1. The path printed is [ 'levels', 0, 'screen' ]. In other words, as printed, it points to the enclosing dictionary, not the transformBackground property.
  2. The "DELETING" prints as expected the property I'm looking for, but the "BUT REALLY DELETING" is different and contains much more content— it does appear to encompass the entire dictionary stored under "screen".
  3. After applying the edits, jsondiff shows this change for the output file: [{"path": "/levels/0", "value": {"color": [0.3, 0.37, 1], "png": "level/test/trigger/trigger.png"}, "op": "replace"}]. In other words, it shows the "screen" property and its value were deleted from /levels/0.

Analysis

I am able to get the "expected behavior" I desire by, instead of passing at.node to getNodePath in the "PERFORM OPERATIONS HERE" section, passing in either of its children. If I do this I get the path [ 'levels', 0, 'screen', 'transformBackground' ] as expected and this is indeed the content that gets removed. However this feels wierd, and additionally it contradicts the documentation. The comment/doc for modify says: " * @param path The path of the value to change. The path represents either to the document root, a property or an array item." This implies modify should accept property nodes.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia con la riproduzione in src/app.ts ed esegui npm install && npm run build, seguito dal comando node documentato. Esamina i punti di ingresso getNodePath e modify rispetto al comportamento dei percorsi delle proprietà descritto nell’issue. Il lavoro è completato quando findNodeAtLocation(tree, getNodePath(node)) restituisce il nodo della proprietà e modify rimuove solo quella proprietà.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
typescript
Ambito
tooling
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
42/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.