TypeFox / yang-vscode

A VS Code Extension for YANG
Apache License 2.0
26 stars 10 forks source link

Cannot disable bad-import-yang-version #40

Open Sparrow1029 opened 3 years ago

Sparrow1029 commented 3 years ago

Thank you for making this extension!

I have YANG models that I am using with Cisco NSO (Network Services Orchestrator), and cannot disable bad-import-yang-version error.

I have set up my ~/.yang/yang.settings file like this:

{
  "yangPath": "/Path/to/ietf:/tailf/modules",
  "diagnostic": {
    "bad-import-yang-version": "ignore",
    "ambiguous-import": "ignore"
  }
}

I have also tried placing this same configuration into yang.settings at my project root.

Setting "ambiguous-import" to "ignore" works just fine, as those warnings are now disabled.

I currently have some "duplicate-name" errors in my YANG, and as an experiment, successfully changed/disabled those errors using yang.settings as well.

The bad-import-yang-version errors do not disappear until I explicitly put

module my-module {
    yang-version 1.1;

at the top of the module (which isn't usually necessary for Cisco NSO to infer the version).

Adding the explicit yang-version leaf at the top of the modules isn't a huge pain, but wanted to report this behavior. I am wondering if it is a bug, or if I should be using some other configuration.

vk496 commented 2 years ago

Same for me

dhuebner commented 2 years ago

@Sparrow1029 Empty yang-version assumes version 1.0

This kind of validation bad-import-yang-version and bad-include-yang-version is currently explicitly set to error and is not configurable. I could change that, but I'm not sure yet if it can break the consistency of the LS.

https://datatracker.ietf.org/doc/html/rfc7950#section-12

esmasth commented 6 months ago

Hello @dhuebner, I have a related issue on TypeFox/yang-lsp#228 where it is requested to refine the diagnostic to match the RFC, as it is allowed to import yang-version 1.1 models from yang-version 1.0 models as long as the import is not with a revision-date.

Also, this ticket was helpful, since I had struggled in isolation when trying to downgrade bad-import-yang-version to a warning severity, due to the false positive against the RFC compliant import behavior.

dhuebner commented 6 months ago

@esmasth We need to make severity for bad-import-yang-version and bad-include-yang-version validation configurable. Unfortunately I don't have much time currently to implement this now. Maybe you could talk to @RimShao to change the priorities, so we can work on this issue.

esmasth commented 6 months ago

Thanks @dhuebner, I have discussed with Rim and this is rather an independent engagement from my side hence it should not not interfere. I may provide a draft PR for consideration, if welcome, for TypeFox/yang-lsp#228 when I get my development environment up and running.

dhuebner commented 6 months ago

Making validation configurable bei the user is pretty easy, see: https://github.com/TypeFox/yang-lsp/pull/237#issuecomment-2009212302

esmasth commented 5 months ago

Thanks @dhuebner . The other cases of using error(...) in yang-lsp seemed to be consistent with MUST/SHALL/... statements in the YANG RFCs on a quick check. Unless there's a motivation brought forth on why bad-import-yang-version and bad-include-yang-version need to be worked around, especially after the fix for TypeFox/yang-lsp#228, it would seem to me that configurability could be limited to SHOULD/RECOMMENDED/..., best practices, and inconsistent spec/implementation domain.

dhuebner commented 5 months ago

@esmasth If you already have a list of SHOULD/RECOMMENDED/ validation to make configurable, maybe you can provide a PR for that?

esmasth commented 5 months ago

@dhuebner it would need a dedicated exercise to comb through the layers of RFC text to create that definitive list. I don't have that readily available for contribution, sorry.