Skip to content

[Fix] the problem that sample rules could not be loaded and registerd normally#30

Merged
guwirth merged 1 commit intoSonarOpenCommunity:masterfrom
smileQiny:master
Mar 13, 2024
Merged

[Fix] the problem that sample rules could not be loaded and registerd normally#30
guwirth merged 1 commit intoSonarOpenCommunity:masterfrom
smileQiny:master

Conversation

@smileQiny
Copy link
Copy Markdown

I did the following checks and tests to verify:
The latest https://github.com/SonarOpenCommunity/sonar-cxx (commit id 04508e8446e236e9044c5411229be7dee1554355) and the latest https://github.com/SonarOpenCommunity/cxx-custom-checks-example-plugin (commit id 55b0738 812c1a26aecbbed4496063), they is not work. I understand that we need to set their html prompt information for each rule.
image

…d normally

Signed-off-by: qinyong <qiny15@chinatelecom.cn>
@smileQiny
Copy link
Copy Markdown
Author

The following are the repair verification results:
Rule loading:
image

Server display:
image

Example rule functionality:
image

@smileQiny
Copy link
Copy Markdown
Author

@guwirth
hi guwirth,
Can you review this pr?

@guwirth
Copy link
Copy Markdown
Contributor

guwirth commented Mar 9, 2024

Hi @smileQiny,

thanks for testing.

In your first PR you were providing a solution for loading the annotation from the HTML file. The reason why I removed it was, that I thought the solution/code should be in the cxx plugin and not in every custom cxx plugin.

Using description is one option, but also the second with the HTML should work.
I will merge this and will look for an HTML support.

Second thing is that the custom plugin should have also a version number, it’s null.

Regards,

@guwirth
Copy link
Copy Markdown
Contributor

guwirth commented Mar 9, 2024

that’s the code I mean should be somehow working if derived from CustomCxxRulesDefinition:

 @Override
  public void define(RulesDefinition.Context context) {
    var repo = context.createRepository(repositoryKey(), getLanguage().getKey())
            .setName(repositoryName());

    // Load metadata from check classes' annotations
    new AnnotationBasedRulesDefinition(repo, getLanguage().getKey()).addRuleClasses(false,
            Arrays.asList(checkClasses()));

    // Optionally override html description from annotation with content from html files
    repo.rules().forEach(rule -> rule.setHtmlDescription(loadResource("/org/sonar/l10n/cxx/rules/cxx/" + rule.key() + ".html")));
    repo.done();
  }

  private String loadResource(String path) {
    URL resource = getClass().getResource(path);
    if (resource == null) {
      throw new IllegalStateException("Resource not found: " + path);
    }
    ByteArrayOutputStream result = new ByteArrayOutputStream();
    try (InputStream in = resource.openStream()) {
      byte[] buffer = new byte[1024];
      for (int len = in.read(buffer); len != -1; len = in.read(buffer)) {
        result.write(buffer, 0, len);
      }
      return new String(result.toByteArray(), StandardCharsets.UTF_8);
    } catch (IOException e) {
      throw new IllegalStateException("Failed to read resource: " + path, e);
    }
  }

@guwirth
Copy link
Copy Markdown
Contributor

guwirth commented Mar 9, 2024

@guwirth
Copy link
Copy Markdown
Contributor

guwirth commented Mar 11, 2024

@smileQiny : Not sure, but I have the feeling that the current code tries to read the .HTML from the cxx plugin and not the custom plugin?

https://github.com/SonarOpenCommunity/sonar-cxx/blob/04508e8446e236e9044c5411229be7dee1554355/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/CustomCxxRulesDefinition.java#L35

public void define(RulesDefinition.Context context) {
    var repo = context.createRepository(repositoryKey(), CxxLanguage.KEY)
      .setName(repositoryName());

    // Load metadata from check classes' annotations
    new AnnotationBasedRulesDefinition(repo, CxxLanguage.KEY).addRuleClasses(false,
                                                                             Arrays.asList(checkClasses()));

    repo.done();
  }

https://github.com/SonarOpenCommunity/sonar-cxx/blob/04508e8446e236e9044c5411229be7dee1554355/cxx-squid-bridge/src/main/java/org/sonar/cxx/squidbridge/annotations/AnnotationBasedRulesDefinition.java#L87

public class AnnotationBasedRulesDefinition {
...
  public AnnotationBasedRulesDefinition(NewRepository repository, String languageKey) {
    this.repository = repository;
    this.languageKey = languageKey;
    var externalDescriptionBasePath = String.format("/org/sonar/l10n/%s/rules/%s", languageKey, repository.key());
    this.externalDescriptionLoader = new ExternalDescriptionLoader(repository, externalDescriptionBasePath);
  }
...
public void addRuleClasses(boolean failIfSqaleNotFound, boolean failIfNoExplicitKey, Iterable<Class> ruleClasses) {
    new RulesDefinitionAnnotationLoader().load(repository, Iterables.toArray(ruleClasses, Class.class));
    List<NewRule> newRules = Lists.newArrayList();
    for (var ruleClass : ruleClasses) {
      var rule = newRule(ruleClass, failIfNoExplicitKey);
      externalDescriptionLoader.addHtmlDescription(rule);
      rule.setTemplate(AnnotationUtils.getAnnotation(ruleClass, RuleTemplate.class) != null);
      if (!isSqaleAnnotated(ruleClass) && failIfSqaleNotFound) {
        throw new IllegalArgumentException("No SqaleSubCharacteristic annotation was found on " + ruleClass);
      }
      try {
        setupSqaleModel(rule, ruleClass);
      } catch (RuntimeException e) {
        throw new IllegalArgumentException("Could not setup SQALE model on " + ruleClass, e);
      }
      newRules.add(rule);
    }
    setupExternalNames(newRules);
  }
...
}

https://github.com/SonarOpenCommunity/sonar-cxx/blob/04508e8446e236e9044c5411229be7dee1554355/cxx-squid-bridge/src/main/java/org/sonar/cxx/squidbridge/rules/ExternalDescriptionLoader.java#L36

public class ExternalDescriptionLoader {

  private final String resourceBasePath;

  public ExternalDescriptionLoader(NewRepository repository, String resourceBasePath) {
    this.resourceBasePath = resourceBasePath;
  }

  public static void loadHtmlDescriptions(NewRepository repository, String languageKey) {
    var loader = new ExternalDescriptionLoader(repository, languageKey);
    for (var newRule : repository.rules()) {
      loader.addHtmlDescription(newRule);
    }
  }

  public void addHtmlDescription(NewRule rule) {
    URL resource = ExternalDescriptionLoader.class.getResource(resourceBasePath + "/" + rule.key() + ".html");
    if (resource != null) {
      addHtmlDescription(rule, resource);
    }
  }

  @VisibleForTesting
  void addHtmlDescription(NewRule rule, URL resource) {
    try {
      rule.setHtmlDescription(Resources.toString(resource, Charsets.UTF_8));
    } catch (IOException e) {
      throw new IllegalStateException("Failed to read: " + resource, e);
    }
  }

}

@guwirth guwirth added the bug label Mar 13, 2024
@guwirth guwirth merged commit e1d4e2e into SonarOpenCommunity:master Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants