Design Guidelines

[guwirth]

@ivangalkin @Bertk @jmecosta
Please review and complete: https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Design-Guidelines

Like to finish this before we continue with #1638 & #1650.

[ivangalkin]

Hi @guwirth,

I really like the guidelines. Some minor comments from my side:

The plugin expects to be fed with syntactically correct code. This is a conscious design decision: we do not want to reimplement a compiler.

The first sentence goes in the right direction. The wording of the 2nd sentence is not clear to me. We obviously reimplement a parser (preprocessor + lexer + the language grammar). So what you probably mean is something like: We don't aim to check syntax/semantic by means of our parser. Parsing errors shouldn't be reported as technical debt in terms of SonarQube. Am I right?

erroneous reports; we support a tolerant and strict error handling mode (sonar.cxx.errorRecoveryEnabled)

AFAIK we don't have such feature for the external reports (yet). For the external reports we run in the (hardcoded) strict mode. W.r.t. the source code parsing we run in the tolerant mode by default. I like these defaults - it's easy to parse the external reports, so the strict mode makes sense here. Parsing of the source code is hard, so we should be tolerant to [our own] parsing errors.

In my opinion it would be wrong to overload the property sonar.cxx.errorRecoveryEnabled. For external reports I think we need an extra flag. Something like sonar.cxx.importErrorRecoveryEnabled or such.

Minor: I am not relay sure about the word "tolerant" ("strict mode" vs "tolerant mode"). I would prefer something like "relaxed mode" or "continue-on-error mode" or something better. But that's a matter of taste.

[guwirth]

@ivangalkin

The wording of the 2nd sentence is not clear to me

Think parser and compiler are not the same thing. We read tokens, match it against a grammar and create an AST. We do for example no semantic analysis. The sentence should express somehow that our goal is not to implement a C++ syntax or semantic checker. This is what a compiler is already doing. We also don’t like to create syntax and semantic errors and warnings. This is also what compilers are already doing.

AFAIK we don't have such feature for the external reports

Most sensors are using CxxUtils.validateRecovery
https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxUtils.java#L84

[guwirth]

@ivangalkin

Better?

• the plugin expects to be fed with syntactically correct code
  • use an external compiler to get detailed syntax and semantic issues

[guwirth]

@ivangalkin

In my opinion it would be wrong to overload the property sonar.cxx.errorRecoveryEnabled

Thinking about this the question is if we need a flag for the source code? For source code it should always be errorRecoveryEnabled=True. Remove the other case.
https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-squid/src/main/java/org/sonar/cxx/parser/CxxGrammarImpl.java#L402

[ivangalkin]

@guwirth

Better?

yes, thank you.