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

Feature request: Add JSON path parameter to JSONVisitor functions #50

Closed
Marcono1234 opened this issue Aug 31, 2021 · 3 comments · Fixed by #62
Closed

Feature request: Add JSON path parameter to JSONVisitor functions #50

Marcono1234 opened this issue Aug 31, 2021 · 3 comments · Fixed by #62
Labels
feature-request Request for new features or functionality
Milestone

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Aug 31, 2021

Rationale

The JSONVisitor is quite useful for validating the content of a JSON document (and getting locations of incorrect data). However, currently it is rather cumbersome to find out at which JSON path in the document the parser is when a visitor function is called.

Suggestion

It would therefore be good if some of the JSONVisitor functions had an extra parameter for the JSONPath of the currently visited element:

  • onObjectBegin, onArrayBegin, onLiteralValue
  • onObjectProperty: Here the property which is visited should probably not be part of the JSON path

For the other functions adding a JSONPath would probably either not add much value (onObjectEnd, onSeparator, onArrayEnd), or its behavior cannot be defined very well (onComment, onError).

However, because JSONPath is a mutable array (and is therefore modified when the parser proceeds), it would be sanest to only hand out copies of it to the visitor functions. A performant way in which this could be implemented is to have a supplier function of JSON path so the copy is only created when the user actually uses the JSON path. For example:

export interface JSONVisitor {
    onObjectBegin?: (
        offset: number,
        length: number,
        startLine: number,
        startCharacter: number,
        pathSupplier: () => JSONPath
    ) => void;

    ...
}

Based on the TypeScript documentation adding an extra function parameter should not be an issue.

What do yo think?

Workaround

Users implementing JSONVisitor can manually maintain a JSON path array, updating it in the respective visitor functions.

Demo implementation (might contain bugs)
const path = []

jsoncParser.visit(jsonContent, {
    onError(error, offset, length, startLine, startCharacter) {
        // ... error handling
    },
    onArrayBegin() {
        path.push(0)
    },
    onArrayEnd() {
        path.pop()
    },
    onSeparator() {
        // Check if inside of array (and therefore path element is array index)
        if (typeof path[path.length - 1] === "number") {
            path[path.length - 1]++
        }
    },
    onObjectBegin() {
        // Push temporary placeholder for object property names
        path.push(undefined)
    },
    onObjectEnd() {
        path.pop()
    },
    onObjectProperty(property, offset, length, startLine, startCharacter) {
        path[path.length - 1] = property
    },
    onLiteralValue(value, offset, length, startLine, startCharacter) {
        // ... process value
    }
})
@mcclure
Copy link

mcclure commented Aug 31, 2021

I think if this existed it would address my issue #48.

@aeschli aeschli added the feature-request Request for new features or functionality label Oct 25, 2021
@aeschli aeschli added this to the Backlog milestone Oct 25, 2021
@aeschli
Copy link
Contributor

aeschli commented Oct 25, 2021

I'm open for a PR, if you are interested.

@Marcono1234
Copy link
Contributor Author

Have created #62

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants