[clang-tidy] Generates invalid code - adds invalid escapes to string literals

[H-G-Hristov]

The following was automatically fixed by Clang-Tidy and reformatted with clang-format:

void Config::CheckModified() {
    struct stat fileinfo;
    time_t mytime;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    } else {
        mytime = fileinfo.st_mtime;
        if (mytime > m_LastModifiedTime) {
            snprintf(Buf, MAX_FILENAME, "%s", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, "CheckModified() Reloading Configuration [%s]\n", Buf);
            ResetState();
            LoadConfig(Buf);
        }
    }
}

Invalid output:

void Config::CheckModified() {
    struct stat fileinfo;
    time_t mytime = 0;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    }
    mytime = fileinfo.st_mtime;
    if (mytime > m_LastModifiedTime) {
            snprintf(Buf, MAX_FILENAME, \"%s\", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, \"CheckModified() Reloading Configuration [%s]\
\", Buf);
            ResetState();
            LoadConfig(Buf);
    }
}

[H-G-Hristov]

I used VSCode's plugin to fix the clang-tidy issues automatically: https://github.com/microsoft/vscode-cpptools

[EugeneZelenko]

Could you please run tools separately to find which one causes problem?

[Zingam]

It's clang-tidy. Please check the the screenshot - Line 45. It's an unformatted output.
If necessary I'll try to create a minimal repro.

I think VSCode includes clang-tidy 14. I'm running the VSCode on Ubuntu 22.04/INTEL.

[Zingam]

At the top I copy and pasted the before and after fixes. Please note:

1. Param: "%s" -> \"%s\"
2. Comments

// fprintf(stderr, "CheckModified() Reloading Configuration [%s]\n", Buf);

->

// fprintf(stderr, \"CheckModified() Reloading Configuration [%s]\
\", Buf);

[llvmbot]

@llvm/issue-subscribers-clang-tidy

[Zingam]

It would be also nice when removing:

} else {
   return x;
}

To put the return on a new line and not as it is on the screenshot.

[H-G-Hristov]

---
AnalyzeTemporaryDtors: false
FormatStyle:           none
HeaderFilterRegex:     'agent/[^/]*\.(hpp|cpp|hxx|cxx)$|library/[^/]*\.(hpp|cpp|hxx|cxx)$|initial-validate/[^/]*\.(hpp|cpp|hxx|cxx)$|modules/[^/]*\.(hpp|cpp|hxx|cxx)$'
User:                  user
WarningsAsErrors:      false
# Checks order:
# 1. Disable all default checks `-*`
# 2. Enable all desired checks `-<check_name_group>-*`
# 3. Disable specific checks with `-<check_name>`
Checks: >
  -*,
  boost-*,
  bugprone-*,
  cert-*,
  clang-analyzer-*,
  clang-diagnostic-*,
  concurrency-*,
  cppcoreguidelines-*,
  google-*,
  hicpp-*,
  misc-*,
  modernize-*,
  performance-*,
  portability-*,
  readability-*,
  -modernize-use-trailing-return-type,
  -readability-redundant-access-specifiers,
CheckOptions:
  - key:             cert-dcl16-c.NewSuffixes
    value:           'L;LL;LU;LLU'
  - key:             cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField
    value:           'false'
  - key:             cert-str34-c.DiagnoseSignedUnsignedCharComparisons
    value:           'false'
  - key:             google-readability-namespace-comments.ShortNamespaceLines
    value:           '10'
  - key:             cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors
    value:           'true'
  - key:             cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
    value:           'true' 
  - key:             cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
    value:           'true'
  - key:             google-readability-braces-around-statements.ShortStatementLines
    value:           '1'
  - key:             google-readability-function-size.StatementThreshold
    value:           '800'
  - key:             google-readability-namespace-comments.SpacesBeforeComments
    value:           '2'
  - key:             llvm-else-after-return.WarnOnConditionVariables
    value:           'false'
  - key:             llvm-else-after-return.WarnOnUnfixable
    value:           'false'
  - key:             llvm-qualified-auto.AddConstToQualified
    value:           'false'
  - key:             misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
    value:           'true'
  - key:             modernize-loop-convert.MaxCopySize
    value:           '16'
  - key:             modernize-loop-convert.MinConfidence
    value:           reasonable
  - key:             modernize-loop-convert.NamingStyle
    value:           CamelCase
  - key:             modernize-pass-by-value.IncludeStyle
    value:           llvm
  - key:             modernize-replace-auto-ptr.IncludeStyle
    value:           llvm
  - key:             modernize-use-nullptr.NullMacros
    value:           'NULL'
...

Applying the above on:

#define MAX_FILENAME 2
char* szConfigName;

void ResetState() {}
void LoadConfig(char** buf) {(void) buf; }

void CheckModified() {
    struct stat fileinfo;
    time_t mytime;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    } else {
        mytime = fileinfo.st_mtime;
        if (mytime > 1) {
            snprintf(Buf, MAX_FILENAME, "%s", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, "CheckModified() Reloading Configuration [%s]\n", Buf);
            ResetState();
            LoadConfig(Buf);
        }
    }
}

produces:

#define MAX_FILENAME 2
char* szConfigName;

void ResetState() {}
void LoadConfig(char** buf) {(void) buf; }

void CheckModified() {
    struct stat fileinfo;
    time_t mytime = 0;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    }         mytime = fileinfo.st_mtime;
        if (mytime > 1) {
            snprintf(Buf, MAX_FILENAME, \"%s\", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, \"CheckModified() Reloading Configuration [%s]\
\", Buf);
            ResetState();
            LoadConfig(Buf);
        }
   
}

[njames93]

What's the warning that clang tidy is emitting for that fix?

[H-G-Hristov]

I'm just selecting a Fix all code analysis problems from the menu. This a minimal CMake project that reproduces this on my machine:

Archive.zip

I'll post the same here microsoft/vscode-cpptools#9683 In case it is a VSCode issue.

[H-G-Hristov]

What's the warning that clang tidy is emitting for that fix?

More specific - now I selected to fix: readability-else-after-return.
Looking at the previously posted screenshot above. It maybe related to this fix because it happened in another context too.

[H-G-Hristov]

@njames93 I installed clang-tidy locally and run: clang-tidy -fix main.cpp and got a different result. So this maybe a VSCode implementation issue. Let's see what the VSCode team will say about it.

[sean-mcmanus]

Yeah, there was a bug with our processing of the clang-tidy fixes.yaml output -- so I think this issue can be closed.