From e88ba5275489c28f9a9851dc575efe670fd8ec14 Mon Sep 17 00:00:00 2001 From: lucica28 <57060141+lucica28@users.noreply.github.com> Date: Thu, 25 May 2023 10:47:30 +0300 Subject: [PATCH] replace libdparse in exception check (#68) --- src/dscanner/analysis/pokemon.d | 92 +++++++++------------------------ src/dscanner/analysis/run.d | 10 ++-- 2 files changed, 29 insertions(+), 73 deletions(-) diff --git a/src/dscanner/analysis/pokemon.d b/src/dscanner/analysis/pokemon.d index 172999c..a588379 100644 --- a/src/dscanner/analysis/pokemon.d +++ b/src/dscanner/analysis/pokemon.d @@ -6,11 +6,8 @@ module dscanner.analysis.pokemon; import std.stdio; -import dparse.ast; -import dparse.lexer; import dscanner.analysis.base; import dscanner.analysis.helpers; -import dsymbol.scope_ : Scope; /** * Checks for Pokémon exception handling, i.e. "gotta' catch 'em all". @@ -23,62 +20,27 @@ import dsymbol.scope_ : Scope; * } * --- */ -final class PokemonExceptionCheck : BaseAnalyzer +extern(C++) class PokemonExceptionCheck(AST) : BaseAnalyzerDmd { + mixin AnalyzerInfo!"exception_check"; + alias visit = BaseAnalyzerDmd.visit; + + extern(D) this(string fileName, bool skipTests = false) + { + super(fileName, skipTests); + } + + override void visit(AST.Catch c) + { + if (c.type.isTypeIdentifier().ident.toString() == "Error" || + c.type.isTypeIdentifier().ident.toString() == "Throwable") + addErrorMessage(cast(ulong) c.loc.linnum, cast(ulong) c.loc.charnum, + KEY, MESSAGE); + } + +private: enum MESSAGE = "Catching Error or Throwable is almost always a bad idea."; enum string KEY = "dscanner.suspicious.catch_em_all"; - mixin AnalyzerInfo!"exception_check"; - - alias visit = BaseAnalyzer.visit; - - this(BaseAnalyzerArguments args) - { - super(args); - } - - override void visit(const LastCatch lc) - { - addErrorMessage(lc.tokens[0], KEY, MESSAGE); - lc.accept(this); - } - - bool ignoreType = true; - - override void visit(const Catch c) - { - ignoreType = false; - c.type.accept(this); - ignoreType = true; - - c.accept(this); - } - - override void visit(const Type2 type2) - { - if (ignoreType) - return; - - if (type2.type !is null) - { - type2.type.accept(this); - return; - } - - if (type2.typeIdentifierPart.typeIdentifierPart !is null) - { - return; - } - const identOrTemplate = type2.typeIdentifierPart.identifierOrTemplateInstance; - if (identOrTemplate.templateInstance !is null) - { - return; - } - if (identOrTemplate.identifier.text == "Throwable" - || identOrTemplate.identifier.text == "Error") - { - addErrorMessage(identOrTemplate, KEY, MESSAGE); - } - } } unittest @@ -87,7 +49,7 @@ unittest StaticAnalysisConfig sac = disabledConfig(); sac.exception_check = Check.enabled; - assertAnalyzerWarnings(q{ + assertAnalyzerWarningsDMD(q{ void testCatch() { try @@ -106,23 +68,15 @@ unittest { } - catch (Error err) /+ - ^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/ + catch (Error err) // [warn]: Catching Error or Throwable is almost always a bad idea. { } - catch (Throwable err) /+ - ^^^^^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/ + catch (Throwable err) // [warn]: Catching Error or Throwable is almost always a bad idea. { } - catch (shared(Error) err) /+ - ^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/ - { - - } - catch /+ - ^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/ + catch (shared(Error) err) // [warn]: Catching Error or Throwable is almost always a bad idea. { } @@ -130,4 +84,4 @@ unittest }c, sac); stderr.writeln("Unittest for PokemonExceptionCheck passed."); -} +} \ No newline at end of file diff --git a/src/dscanner/analysis/run.d b/src/dscanner/analysis/run.d index 70273af..ed217cf 100644 --- a/src/dscanner/analysis/run.d +++ b/src/dscanner/analysis/run.d @@ -853,10 +853,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName, checks ~= new DuplicateAttributeCheck(args.setSkipTests( analysisConfig.duplicate_attribute == Check.skipTests && !ut)); - if (moduleName.shouldRun!PokemonExceptionCheck(analysisConfig)) - checks ~= new PokemonExceptionCheck(args.setSkipTests( - analysisConfig.exception_check == Check.skipTests && !ut)); - if (moduleName.shouldRun!FloatOperatorCheck(analysisConfig)) checks ~= new FloatOperatorCheck(args.setSkipTests( analysisConfig.float_operator_check == Check.skipTests && !ut)); @@ -1319,6 +1315,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN if (moduleName.shouldRunDmd!(BuiltinPropertyNameCheck!ASTCodegen)(config)) visitors ~= new BuiltinPropertyNameCheck!ASTCodegen(fileName); + if (moduleName.shouldRunDmd!(PokemonExceptionCheck!ASTCodegen)(config)) + visitors ~= new PokemonExceptionCheck!ASTCodegen( + fileName, + config.exception_check == Check.skipTests && !ut + ); + if (moduleName.shouldRunDmd!(BackwardsRangeCheck!ASTCodegen)(config)) visitors ~= new BackwardsRangeCheck!ASTCodegen( fileName,