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

Incorrect line # called out with issue found #625

Open
QualityRobD opened this issue Aug 3, 2021 · 7 comments
Open

Incorrect line # called out with issue found #625

QualityRobD opened this issue Aug 3, 2021 · 7 comments
Labels

Comments

@QualityRobD
Copy link

Describe the bug
I have some sample yaml that has an issue (missing quote) on line 16, however the callout supplied by YamlDotNet is for line 34.

To Reproduce
See below for the sample

Sample

attach_workspace: &attach_workspace
    attach_workspace:
        at: '.'

deploy_nonprod_job: &deploy_nonprod_job
    working_directory: '~/build'
    docker: [ {image: 'test.com'} ]
    steps:
        - *attach_workspace
        - run: 'deploy-to-test'

version: 2

jobs:
    build_test_image:
        working_directory: '~/build
        docker:
            - image: docker:17.05.0-ce-git
        environment:
            DOCKER_IMAGE_NAME: testApp
        steps:
            - run:
                name: Install bash and curl
                command: |
                    apk update
                    apk add -y curl
                    apk add -y bash
                    apk update
                    apk upgrade

    deploy_test:
        environment:
            HAL_TARGETS: '45597'
            HAL_BUILD_FILE: '.hal_build_id_test'

workflows:
    version: 2
    pipeline:
        jobs:        
            - build_test_image
            - deploy_test

output:

(Line: 34, Col: 27, Idx: 820) - (Line: 34, Col: 27, Idx: 820): While scanning a plain scalar value, found invalid mapping.

the exact same yaml on yamllint.com providing the correct line number with the issue:
image

@QualityRobD
Copy link
Author

QualityRobD commented Aug 3, 2021

It's very interesting that the auto-formatter here called out the correct line. I wonder if my implementation of YamlDotNet is incorrect. I'll supply that shortly.

#625 (comment)

@QualityRobD
Copy link
Author

Here is how I am implementing YamlDotNet. Ideally, I would like to point the user to the correct offending line, which I have not been able to do. Right now I am posting a github comment on the first changed line in their Pull Request, which is far from ideal.

namespace TestApp.BusinessLayer.Analysis.Utilities
{
    public class YamlChecker : IYamlChecker
    {
        public void CheckYaml(string fileData, string fileName, int positionNumber, string commitSha, List<ScanResult> issues)
        {
            ValidateYaml(fileData, fileName, positionNumber, commitSha, issues);
        }

        public static void ValidateYaml(string fileData, string fileName, int positionNumber, string commitSha, List<ScanResult> issues)
        {
            try
            {
                var deserializer = new DeserializerBuilder()
                    .IgnoreUnmatchedProperties()
                    .WithNodeTypeResolver(new MapTagsToObject())
                    .Build();
                deserializer.Deserialize<object>(fileData);
            }
            catch (Exception e)
            {
                // capture failed deserialization information
                var result = new ScanResult
                {
                    FileName = fileName,
                    Issue = "SYNTAX ERROR: YAML failed to validate - **please check entire file (not just this line).**<br/>",
                    Commit = commitSha,
                    IssueContext = e.Message,
                    LineNumber = positionNumber,
                    PositionNumber = positionNumber
                };
                issues.Add(result);
            }
        }
    }

    // Provided by YamlDotNet owner to help ignore custom tags for things like MKdocs
    // https://github.com/aaubry/YamlDotNet/issues/615#issuecomment-861575357
    internal class MapTagsToObject : INodeTypeResolver
    {
        public bool Resolve(NodeEvent? nodeEvent, ref Type currentType)
        {
            if (nodeEvent != null && !nodeEvent.Tag.IsEmpty)
            {
                currentType = typeof(object);
                return true;
            }
            return false;
        }
    }
}

@QualityRobD QualityRobD changed the title Incorrect Line called out with issue found Incorrect line called out with issue found Aug 3, 2021
@QualityRobD QualityRobD changed the title Incorrect line called out with issue found Incorrect line # called out with issue found Aug 3, 2021
@yamldotnet
Copy link

attach_workspace: &attach_workspace
    attach_workspace:
        at: '.'

deploy_nonprod_job: &deploy_nonprod_job
    working_directory: '~/build'
    docker: [ {image: 'test.com'} ]
    steps:
        - *attach_workspace
        - run: 'deploy-to-test'

version: 2

jobs:
    build_test_image:
        working_directory: '~/build
        docker:
            - image: docker:17.05.0-ce-git
        environment:
            DOCKER_IMAGE_NAME: testApp
        steps:
            - run:
                name: Install bash and curl
                command: |
                    apk update
                    apk add -y curl
                    apk add -y bash
                    apk update
                    apk upgrade

    deploy_test:
        environment:
            HAL_TARGETS: '45597'
            HAL_BUILD_FILE: '.hal_build_id_test'

workflows:
    version: 2
    pipeline:
        jobs:        
            - build_test_image
            - deploy_test

This YAML snippet appears to be invalid.
The following errors were identifier by the reference parser:

  • Line 16, char 36: Unexpected '\x0a'

1 similar comment
@yamldotnet
Copy link

attach_workspace: &attach_workspace
    attach_workspace:
        at: '.'

deploy_nonprod_job: &deploy_nonprod_job
    working_directory: '~/build'
    docker: [ {image: 'test.com'} ]
    steps:
        - *attach_workspace
        - run: 'deploy-to-test'

version: 2

jobs:
    build_test_image:
        working_directory: '~/build
        docker:
            - image: docker:17.05.0-ce-git
        environment:
            DOCKER_IMAGE_NAME: testApp
        steps:
            - run:
                name: Install bash and curl
                command: |
                    apk update
                    apk add -y curl
                    apk add -y bash
                    apk update
                    apk upgrade

    deploy_test:
        environment:
            HAL_TARGETS: '45597'
            HAL_BUILD_FILE: '.hal_build_id_test'

workflows:
    version: 2
    pipeline:
        jobs:        
            - build_test_image
            - deploy_test

This YAML snippet appears to be invalid.
The following errors were identifier by the reference parser:

  • Line 16, char 36: Unexpected '\x0a'

@EdwardCooke
Copy link
Collaborator

This exposed a larger issue than just showing the incorrect line #. It also showed that multi-line single quote scalars were handled incorrectly. They require a space at the beginning of a line with content, which a double quote does not. Both a single and double were handled the same with regards to that. I'll be creating a PR to fix this when my current PR gets merged in. You can take a look at my PR/test cases if you would like in this commit:

29cf3b7

@EdwardCooke
Copy link
Collaborator

Mostly a note to me. This is caused by a difference between the yaml spec 1.1 and 1.2. Requiring the space at the front will violate the 1.1 spec and most likely break current users that rely on 1.1. This needs to be a feature that is turned on by user code, probably a 1.2 enforcement option. At least until we possibly default to 1.2 parsing. I’ll work on this more when I’ve completed more of the smaller low hanging fruit bugs.

@EdwardCooke
Copy link
Collaborator

After rechecking my thoughts on what was expected in the spec, in 1.1, it is expected that the value be indented by an additional space when the value was already indented.
For example, these 2 are should be, according to the spec, be valid:

a:
 b: 'c
  d'
'hello
world'

We should be good once the PR is approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants